From 5844339165d57d1b7899d2272a37fe23c15f8e3f Mon Sep 17 00:00:00 2001 From: Divyanshu Agrawal Date: Fri, 12 Jun 2020 11:58:27 +0530 Subject: [PATCH 1/9] api: Create binding to get subscription status to a stream. Creates a binding to the endpoint '/users/{user_id}/subscriptions/{stream_id}' that returns whether or not a user is subscribed to a stream. See https://zulip.com/api/get-subscription-status for details. --- src/api/index.js | 2 ++ .../subscriptions/getSubscriptionToStream.js | 21 +++++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 src/api/subscriptions/getSubscriptionToStream.js diff --git a/src/api/index.js b/src/api/index.js index 1f6219f802f..a65263be063 100644 --- a/src/api/index.js +++ b/src/api/index.js @@ -42,6 +42,7 @@ import subscriptionRemove from './subscriptions/subscriptionRemove'; import toggleMuteStream from './subscriptions/toggleMuteStream'; import togglePinStream from './subscriptions/togglePinStream'; import toggleStreamNotifications from './subscriptions/toggleStreamNotifications'; +import getSubscriptionToStream from './subscriptions/getSubscriptionToStream'; import unmuteTopic from './subscriptions/unmuteTopic'; import tryGetFileTemporaryUrl from './tryGetFileTemporaryUrl'; import createUserGroup from './user_groups/createUserGroup'; @@ -97,6 +98,7 @@ export { muteTopic, subscriptionAdd, subscriptionRemove, + getSubscriptionToStream, toggleMuteStream, togglePinStream, toggleStreamNotifications, diff --git a/src/api/subscriptions/getSubscriptionToStream.js b/src/api/subscriptions/getSubscriptionToStream.js new file mode 100644 index 00000000000..1c521d58f7b --- /dev/null +++ b/src/api/subscriptions/getSubscriptionToStream.js @@ -0,0 +1,21 @@ +/* @flow strict-local */ +import type { Auth, ApiResponseSuccess } from '../transportTypes'; +import { apiGet } from '../apiFetch'; + +type ApiResponseSubscriptionStatus = {| + ...ApiResponseSuccess, + is_subscribed: boolean, +|}; + +/** + * Get whether a user is subscribed to a particular stream. + * + * See https://zulip.com/api/get-subscription-status for + * documentation of this endpoint. + */ +export default ( + auth: Auth, + userId: number, + streamId: number, +): Promise => + apiGet(auth, `users/${userId}/subscriptions/${streamId}`); From de6c66cebcc01f516b52ad9a5c70b246a668d450 Mon Sep 17 00:00:00 2001 From: Divyanshu Agrawal Date: Mon, 11 May 2020 13:28:43 +0530 Subject: [PATCH 2/9] compose: Create component to warn on unsubscribed @-mentionee. Creates a component 'MentionedUserNotSubscribed' that can be used to shows a warning when an @-mentioned user is not subscribed to a stream they are mentioned in, with a button to subscribe them. --- src/message/MentionedUserNotSubscribed.js | 84 +++++++++++++++++++++++ static/translations/messages_en.json | 1 + 2 files changed, 85 insertions(+) create mode 100644 src/message/MentionedUserNotSubscribed.js diff --git a/src/message/MentionedUserNotSubscribed.js b/src/message/MentionedUserNotSubscribed.js new file mode 100644 index 00000000000..02e3dee48b9 --- /dev/null +++ b/src/message/MentionedUserNotSubscribed.js @@ -0,0 +1,84 @@ +/* @flow strict-local */ + +import React, { PureComponent } from 'react'; +import { View, TouchableOpacity, StyleSheet } from 'react-native'; + +import type { Auth, Stream, Dispatch, UserOrBot, Subscription } from '../types'; +import { connect } from '../react-redux'; +import * as api from '../api'; +import { ZulipButton, Label } from '../common'; +import { getAuth } from '../selectors'; + +type SelectorProps = {| + auth: Auth, +|}; + +type Props = $ReadOnly<{| + user: UserOrBot, + stream: Subscription | {| ...Stream, in_home_view: boolean |}, + onDismiss: (user: UserOrBot) => void, + + dispatch: Dispatch, + ...SelectorProps, +|}>; + +const styles = StyleSheet.create({ + mentionedUserNotSubscribed: { + flexDirection: 'row', + alignItems: 'center', + justifyContent: 'space-around', + backgroundColor: 'hsl(40, 100%, 60%)', // Material warning-color + paddingHorizontal: 16, + paddingVertical: 8, + borderTopWidth: 1, + borderTopColor: 'orange', + }, + mentionedUserNotSubscribedText: { + flex: 1, + color: 'white', + }, + mentionedUserNotSubscribedButton: { + backgroundColor: 'orange', + padding: 6, + }, +}); + +class MentionedUserNotSubscribed extends PureComponent { + subscribeToStream = () => { + const { auth, stream, user } = this.props; + api.subscriptionAdd(auth, [{ name: stream.name }], [user.email]); + this.handleDismiss(); + }; + + handleDismiss = () => { + const { user, onDismiss } = this.props; + onDismiss(user); + }; + + render() { + const { user } = this.props; + + return ( + + + + + ); + } +} + +export default connect((state, props) => ({ + auth: getAuth(state), +}))(MentionedUserNotSubscribed); diff --git a/static/translations/messages_en.json b/static/translations/messages_en.json index 3385814a6e1..3a22d710e21 100644 --- a/static/translations/messages_en.json +++ b/static/translations/messages_en.json @@ -106,6 +106,7 @@ "Enable notifications": "Enable notifications", "Jot down something": "Jot down something", "Message {recipient}": "Message {recipient}", + "{username} will not be notified unless you subscribe them to this stream.": "{username} will not be notified unless you subscribe them to this stream.", "Send private message": "Send private message", "View private messages": "View private messages", "(This user has been deactivated)": "(This user has been deactivated)", From b02392284b16b8544e7b479a54900203f4fb32ac Mon Sep 17 00:00:00 2001 From: Divyanshu Agrawal Date: Mon, 15 Jun 2020 13:34:22 +0530 Subject: [PATCH 3/9] autocomplete: Pass more information to callback. Adds more parameters to the callback `onAutocomplete` - namely 'completion' and 'lastWordPrefix', which can be used by the handler to perform more complex processing, if needed. --- src/autocomplete/AutocompleteView.js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/autocomplete/AutocompleteView.js b/src/autocomplete/AutocompleteView.js index b7a1f3ba157..9e4bda4adc7 100644 --- a/src/autocomplete/AutocompleteView.js +++ b/src/autocomplete/AutocompleteView.js @@ -19,14 +19,24 @@ type Props = $ReadOnly<{| isFocused: boolean, text: string, selection: InputSelection, - onAutocomplete: (input: string) => void, + + /** + * The callback that is called when the user taps on any of the suggested items. + * + * @param input The text entered by the user, modified to include the autocompletion. + * @param completion The suggestion selected by the user. Includes markdown formatting, + but not the prefix. Eg. **FullName**, **StreamName**. + * @param lastWordPrefix The type of the autocompletion - valid values are keys of 'prefixToComponent'. + */ + onAutocomplete: (input: string, completion: string, lastWordPrefix: string) => void, |}>; export default class AutocompleteView extends PureComponent { handleAutocomplete = (autocomplete: string) => { const { text, onAutocomplete, selection } = this.props; + const { lastWordPrefix } = getAutocompleteFilter(text, selection); const newText = getAutocompletedText(text, autocomplete, selection); - onAutocomplete(newText); + onAutocomplete(newText, autocomplete, lastWordPrefix); }; render() { From a428d55aff4f12f78c3efb7f036961cc48308284 Mon Sep 17 00:00:00 2001 From: Divyanshu Agrawal Date: Mon, 15 Jun 2020 17:34:26 +0530 Subject: [PATCH 4/9] compose: Create component to process unsubscribed mentions. Creates a component, `MentionWarnings`, that processes and renders warnings when a user that's not subscribed to a stream is mentioned. This is an uncontrolled component, and will be accessed using refs. Therefore, passing ` { withRef: true }` option in the `connect` function is required. --- src/compose/MentionWarnings.js | 186 +++++++++++++++++++++++++++ static/translations/messages_en.json | 1 + 2 files changed, 187 insertions(+) create mode 100644 src/compose/MentionWarnings.js diff --git a/src/compose/MentionWarnings.js b/src/compose/MentionWarnings.js new file mode 100644 index 00000000000..1a99ece0511 --- /dev/null +++ b/src/compose/MentionWarnings.js @@ -0,0 +1,186 @@ +/* @flow strict-local */ + +import React, { PureComponent } from 'react'; +import { connect } from 'react-redux'; + +import type { Auth, Stream, Dispatch, Narrow, UserOrBot, Subscription, GetText } from '../types'; +import { TranslationContext } from '../boot/TranslationProvider'; +import { getActiveUsersById, getAuth } from '../selectors'; +import { isPrivateNarrow } from '../utils/narrow'; +import * as api from '../api'; +import { showToast } from '../utils/info'; + +import AnimatedScaleComponent from '../animation/AnimatedScaleComponent'; +import MentionedUserNotSubscribed from '../message/MentionedUserNotSubscribed'; + +type State = {| + unsubscribedMentions: Array, +|}; + +type SelectorProps = {| + auth: Auth, + usersById: Map, +|}; + +type Props = $ReadOnly<{| + narrow: Narrow, + stream: Subscription | {| ...Stream, in_home_view: boolean |}, + + dispatch: Dispatch, + ...SelectorProps, +|}>; + +class MentionWarnings extends PureComponent { + static contextType = TranslationContext; + context: GetText; + + state = { + unsubscribedMentions: [], + }; + + /** + * Tries to parse a user object from an @-mention. + * + * @param completion The autocomplete option chosend by the user. + See JSDoc for AutoCompleteView for details. + */ + getUserFromMention = (completion: string): UserOrBot | void => { + const { usersById } = this.props; + + const unformattedMessage = completion.split('**')[1]; + const [userFullName, userIdRaw] = unformattedMessage.split('|'); + + // We skip user groups, for which autocompletes are of the form + // `**`, and therefore, message.split('**')[1] + // is undefined. + if (unformattedMessage === undefined) { + return undefined; + } + + if (userIdRaw !== undefined) { + const userId = Number.parseInt(userIdRaw, 10); + return usersById.get(userId); + } + + for (const user of usersById.values()) { + if (user.full_name === userFullName) { + return user; + } + } + + return undefined; + }; + + showSubscriptionStatusLoadError = (mentionedUser: UserOrBot) => { + const _ = this.context; + + const alertTitle = _.intl.formatMessage( + { + id: "Couldn't load information about {fullName}", + defaultMessage: "Couldn't load information about {fullName}", + }, + { fullName: mentionedUser.full_name }, + ); + showToast(alertTitle); + }; + + /** + * Check whether the message text entered by the user contains + * an @-mention to a user unsubscribed to the current stream, and if + * so, shows a warning. + * + * This function is expected to be called by `ComposeBox` using a ref + * to this component. + * + * @param completion The autocomplete option chosend by the user. + See JSDoc for AutoCompleteView for details. + */ + handleMentionSubscribedCheck = async (completion: string) => { + const { narrow, auth, stream } = this.props; + const { unsubscribedMentions } = this.state; + + if (isPrivateNarrow(narrow)) { + return; + } + const mentionedUser = this.getUserFromMention(completion); + if (mentionedUser === undefined || unsubscribedMentions.includes(mentionedUser.user_id)) { + return; + } + + let isSubscribed: boolean; + try { + isSubscribed = (await api.getSubscriptionToStream( + auth, + mentionedUser.user_id, + stream.stream_id, + )).is_subscribed; + } catch (err) { + this.showSubscriptionStatusLoadError(mentionedUser); + return; + } + + if (!isSubscribed) { + this.setState(prevState => ({ + unsubscribedMentions: [...prevState.unsubscribedMentions, mentionedUser.user_id], + })); + } + }; + + handleMentionWarningDismiss = (user: UserOrBot) => { + this.setState(prevState => ({ + unsubscribedMentions: prevState.unsubscribedMentions.filter( + (x: number) => x !== user.user_id, + ), + })); + }; + + clearMentionWarnings = () => { + this.setState({ + unsubscribedMentions: [], + }); + }; + + render() { + const { unsubscribedMentions } = this.state; + const { stream, narrow, usersById } = this.props; + + if (isPrivateNarrow(narrow)) { + return null; + } + + const mentionWarnings = []; + for (const userId of unsubscribedMentions) { + const user = usersById.get(userId); + + if (user === undefined) { + continue; + } + + mentionWarnings.push( + , + ); + } + + return ( + + {mentionWarnings} + + ); + } +} + +// $FlowFixMe. TODO: Use a type checked connect call. +export default connect( + state => ({ + auth: getAuth(state), + usersById: getActiveUsersById(state), + }), + null, + null, + { withRef: true }, +)(MentionWarnings); diff --git a/static/translations/messages_en.json b/static/translations/messages_en.json index 3a22d710e21..f365e35ca3d 100644 --- a/static/translations/messages_en.json +++ b/static/translations/messages_en.json @@ -216,6 +216,7 @@ "Sending Message...": "Sending Message...", "Failed to send message": "Failed to send message", "Message sent": "Message sent", + "Couldn't load information about {fullName}": "Couldn't load information about {fullName}", "What's your status?": "What's your status?", "📅 In a meeting": "📅 In a meeting", "🚌 Commuting": "🚌 Commuting", From 20bd98a52b48bf846187f1a840388319a5e71a8b Mon Sep 17 00:00:00 2001 From: Divyanshu Agrawal Date: Mon, 15 Jun 2020 17:54:46 +0530 Subject: [PATCH 5/9] compose: Warn on @-mention when user is not subscribed. Show a warning when @-mentioning a user who is not subscribed to the stream the user was mentioned in, with an option to subscribe them to the stream. Use the created pipeline in the previous commits to enable the same. Fixes: #3373. --- src/compose/ComposeBox.js | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/compose/ComposeBox.js b/src/compose/ComposeBox.js index 07c91982fa9..a389f61703f 100644 --- a/src/compose/ComposeBox.js +++ b/src/compose/ComposeBox.js @@ -16,6 +16,8 @@ import type { Dimensions, CaughtUp, GetText, + Subscription, + Stream, } from '../types'; import { connect } from '../react-redux'; import { withGetText } from '../boot/TranslationProvider'; @@ -35,6 +37,7 @@ import ComposeMenu from './ComposeMenu'; import getComposeInputPlaceholder from './getComposeInputPlaceholder'; import NotSubscribed from '../message/NotSubscribed'; import AnnouncementOnly from '../message/AnnouncementOnly'; +import MentionWarnings from './MentionWarnings'; import { getAuth, @@ -43,6 +46,7 @@ import { getLastMessageTopic, getActiveUsersByEmail, getCaughtUpForNarrow, + getStreamInNarrow, } from '../selectors'; import { getIsActiveStreamSubscribed, @@ -64,6 +68,7 @@ type SelectorProps = {| draft: string, lastMessageTopic: string, caughtUp: CaughtUp, + stream: Subscription | {| ...Stream, in_home_view: boolean |}, |}; type Props = $ReadOnly<{| @@ -116,7 +121,7 @@ class ComposeBox extends PureComponent { messageInput: ?TextInput = null; topicInput: ?TextInput = null; - + mentionWarnings: React$ElementRef = React.createRef(); inputBlurTimeoutId: ?TimeoutID = null; state = { @@ -190,8 +195,15 @@ class ComposeBox extends PureComponent { dispatch(draftUpdate(narrow, message)); }; - handleMessageAutocomplete = (message: string) => { + // See JSDoc on 'onAutocomplete' in 'AutocompleteView.js'. + handleMessageAutocomplete = (message: string, completion: string, lastWordPrefix: string) => { this.setMessageInputValue(message); + + if (lastWordPrefix === '@') { + if (this.mentionWarnings.current) { + this.mentionWarnings.current.getWrappedInstance().handleMentionSubscribedCheck(completion); + } + } }; handleMessageSelectionChange = (event: { +nativeEvent: { +selection: InputSelection } }) => { @@ -261,6 +273,11 @@ class ComposeBox extends PureComponent { dispatch(addToOutbox(this.getDestinationNarrow(), message)); this.setMessageInputValue(''); + + if (this.mentionWarnings.current) { + this.mentionWarnings.current.getWrappedInstance().clearMentionWarnings(); + } + dispatch(sendTypingStop(narrow)); }; @@ -354,6 +371,7 @@ class ComposeBox extends PureComponent { isAdmin, isAnnouncementOnly, isSubscribed, + stream, } = this.props; if (!isSubscribed) { @@ -370,6 +388,7 @@ class ComposeBox extends PureComponent { return ( + ((state, props) => ({ draft: getDraftForNarrow(state, props.narrow), lastMessageTopic: getLastMessageTopic(state, props.narrow), caughtUp: getCaughtUpForNarrow(state, props.narrow), + stream: getStreamInNarrow(state, props.narrow), }))(withGetText(ComposeBox)); From 0f15268b09da31955a6fde9d1cdff90989e7f1fe Mon Sep 17 00:00:00 2001 From: Divyanshu Agrawal Date: Sat, 4 Jul 2020 15:56:23 +0530 Subject: [PATCH 6/9] compose [nfc]: Rename a parameter to 'completedText' for accuracy. Renames the 'message' parameter in the 'handleMessageAutocomplete' method in 'ComposeBox' to 'completedText' for better clarity, so that we don't think of the 'Message' type. --- src/compose/ComposeBox.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/compose/ComposeBox.js b/src/compose/ComposeBox.js index a389f61703f..8980c152603 100644 --- a/src/compose/ComposeBox.js +++ b/src/compose/ComposeBox.js @@ -196,8 +196,12 @@ class ComposeBox extends PureComponent { }; // See JSDoc on 'onAutocomplete' in 'AutocompleteView.js'. - handleMessageAutocomplete = (message: string, completion: string, lastWordPrefix: string) => { - this.setMessageInputValue(message); + handleMessageAutocomplete = ( + completedText: string, + completion: string, + lastWordPrefix: string, + ) => { + this.setMessageInputValue(completedText); if (lastWordPrefix === '@') { if (this.mentionWarnings.current) { From 1ea9e9e1ed90c3f297c2e328c3c928f2891219d5 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 6 Aug 2020 01:55:46 -0700 Subject: [PATCH 7/9] compose [nfc]: Shorten names of some styles. No need to repeat the name of the component. (I think these names are a remnant of a previous version where these styles lived in a more global location.) --- src/message/MentionedUserNotSubscribed.js | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/message/MentionedUserNotSubscribed.js b/src/message/MentionedUserNotSubscribed.js index 02e3dee48b9..501772f95c3 100644 --- a/src/message/MentionedUserNotSubscribed.js +++ b/src/message/MentionedUserNotSubscribed.js @@ -23,7 +23,7 @@ type Props = $ReadOnly<{| |}>; const styles = StyleSheet.create({ - mentionedUserNotSubscribed: { + outer: { flexDirection: 'row', alignItems: 'center', justifyContent: 'space-around', @@ -33,11 +33,11 @@ const styles = StyleSheet.create({ borderTopWidth: 1, borderTopColor: 'orange', }, - mentionedUserNotSubscribedText: { + text: { flex: 1, color: 'white', }, - mentionedUserNotSubscribedButton: { + button: { backgroundColor: 'orange', padding: 6, }, @@ -60,19 +60,15 @@ class MentionedUserNotSubscribed extends PureComponent { return ( - + ); From 387ae00f50d05323bf4c27216390a9d242e5cf61 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 6 Aug 2020 01:45:13 -0700 Subject: [PATCH 8/9] ZulipButton: Offer a textStyle prop. This allows the caller to customize the appearance of the text, where needed e.g. to go along with customizations to the button's background color. --- src/common/ZulipButton.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/common/ZulipButton.js b/src/common/ZulipButton.js index 45a76313c35..4a653a74e22 100644 --- a/src/common/ZulipButton.js +++ b/src/common/ZulipButton.js @@ -1,7 +1,7 @@ /* @flow strict-local */ import React, { PureComponent } from 'react'; import { StyleSheet, Text, View, ActivityIndicator } from 'react-native'; -import type { ViewStyleProp } from 'react-native/Libraries/StyleSheet/StyleSheet'; +import type { TextStyleProp, ViewStyleProp } from 'react-native/Libraries/StyleSheet/StyleSheet'; import TranslatedText from './TranslatedText'; import type { LocalizableText } from '../types'; @@ -62,6 +62,7 @@ const styles = StyleSheet.create({ type Props = $ReadOnly<{| style?: ViewStyleProp, + textStyle?: TextStyleProp, progress: boolean, disabled: boolean, Icon?: SpecificIconType, @@ -78,6 +79,7 @@ type Props = $ReadOnly<{| * have their `secondary` property set to `true`. * * @prop [style] - Style object applied to the outermost component. + * @prop [textStyle] - Style applied to the button text. * @prop [progress] - Shows a progress indicator in place of the button text. * @prop [disabled] - If set the button is not pressable and visually looks disabled. * @prop [Icon] - Icon component to display in front of the button text @@ -110,6 +112,7 @@ export default class ZulipButton extends PureComponent { const textStyle = [ styles.text, disabled ? styles.disabledText : secondary ? styles.secondaryText : styles.primaryText, + this.props.textStyle, ]; const iconStyle = [styles.icon, secondary ? styles.secondaryIcon : styles.primaryIcon]; From 29168ac74de23ffeb235f90297a3cb00bde8b3dd Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 6 Aug 2020 01:59:35 -0700 Subject: [PATCH 9/9] compose: Fix text-color contrast in mention-unsubscribed warning. This color is a pretty light one; it needs a dark text color to get acceptable contrast for legibility. --- src/message/MentionedUserNotSubscribed.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/message/MentionedUserNotSubscribed.js b/src/message/MentionedUserNotSubscribed.js index 501772f95c3..2a77ca3922f 100644 --- a/src/message/MentionedUserNotSubscribed.js +++ b/src/message/MentionedUserNotSubscribed.js @@ -35,12 +35,15 @@ const styles = StyleSheet.create({ }, text: { flex: 1, - color: 'white', + color: 'black', }, button: { backgroundColor: 'orange', padding: 6, }, + buttonText: { + color: 'black', + }, }); class MentionedUserNotSubscribed extends PureComponent { @@ -68,7 +71,12 @@ class MentionedUserNotSubscribed extends PureComponent { }} style={styles.text} /> - + );