Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn on @-mentioning someone who won't see it because not subscribed #4101

Merged
merged 9 commits into from
Aug 6, 2020
2 changes: 2 additions & 0 deletions src/api/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -97,6 +98,7 @@ export {
muteTopic,
subscriptionAdd,
subscriptionRemove,
getSubscriptionToStream,
toggleMuteStream,
togglePinStream,
toggleStreamNotifications,
Expand Down
21 changes: 21 additions & 0 deletions src/api/subscriptions/getSubscriptionToStream.js
Original file line number Diff line number Diff line change
@@ -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<ApiResponseSubscriptionStatus> =>
gnprice marked this conversation as resolved.
Show resolved Hide resolved
apiGet(auth, `users/${userId}/subscriptions/${streamId}`);
14 changes: 12 additions & 2 deletions src/autocomplete/AutocompleteView.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be helpful to have some JSDoc about the interface of the AutocompleteView component, in particular, describing what information is represented by completion and lastWordPrefix here. I don't really understand it just from the variable names, and it shouldn't be necessary (if we can avoid it) to have to inspect the implementation.

|}>;

export default class AutocompleteView extends PureComponent<Props> {
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() {
Expand Down
5 changes: 4 additions & 1 deletion src/common/ZulipButton.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -62,6 +62,7 @@ const styles = StyleSheet.create({

type Props = $ReadOnly<{|
style?: ViewStyleProp,
textStyle?: TextStyleProp,
progress: boolean,
disabled: boolean,
Icon?: SpecificIconType,
Expand All @@ -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
Expand Down Expand Up @@ -110,6 +112,7 @@ export default class ZulipButton extends PureComponent<Props> {
const textStyle = [
styles.text,
disabled ? styles.disabledText : secondary ? styles.secondaryText : styles.primaryText,
this.props.textStyle,
];
const iconStyle = [styles.icon, secondary ? styles.secondaryIcon : styles.primaryIcon];

Expand Down
30 changes: 27 additions & 3 deletions src/compose/ComposeBox.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import type {
Dimensions,
CaughtUp,
GetText,
Subscription,
Stream,
} from '../types';
import { connect } from '../react-redux';
import { withGetText } from '../boot/TranslationProvider';
Expand All @@ -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,
Expand All @@ -43,6 +46,7 @@ import {
getLastMessageTopic,
getActiveUsersByEmail,
getCaughtUpForNarrow,
getStreamInNarrow,
} from '../selectors';
import {
getIsActiveStreamSubscribed,
Expand All @@ -64,6 +68,7 @@ type SelectorProps = {|
draft: string,
lastMessageTopic: string,
caughtUp: CaughtUp,
stream: Subscription | {| ...Stream, in_home_view: boolean |},
|};

type Props = $ReadOnly<{|
Expand Down Expand Up @@ -116,7 +121,7 @@ class ComposeBox extends PureComponent<Props, State> {

messageInput: ?TextInput = null;
topicInput: ?TextInput = null;

mentionWarnings: React$ElementRef<MentionWarnings> = React.createRef();
inputBlurTimeoutId: ?TimeoutID = null;

state = {
Expand Down Expand Up @@ -190,8 +195,19 @@ class ComposeBox extends PureComponent<Props, State> {
dispatch(draftUpdate(narrow, message));
};

handleMessageAutocomplete = (message: string) => {
this.setMessageInputValue(message);
// See JSDoc on 'onAutocomplete' in 'AutocompleteView.js'.
handleMessageAutocomplete = (
completedText: string,
completion: string,
lastWordPrefix: string,
) => {
this.setMessageInputValue(completedText);

if (lastWordPrefix === '@') {
if (this.mentionWarnings.current) {
this.mentionWarnings.current.getWrappedInstance().handleMentionSubscribedCheck(completion);
agrawal-d marked this conversation as resolved.
Show resolved Hide resolved
}
}
};

handleMessageSelectionChange = (event: { +nativeEvent: { +selection: InputSelection } }) => {
Expand Down Expand Up @@ -261,6 +277,11 @@ class ComposeBox extends PureComponent<Props, State> {
dispatch(addToOutbox(this.getDestinationNarrow(), message));

this.setMessageInputValue('');

if (this.mentionWarnings.current) {
this.mentionWarnings.current.getWrappedInstance().clearMentionWarnings();
}

dispatch(sendTypingStop(narrow));
};

Expand Down Expand Up @@ -354,6 +375,7 @@ class ComposeBox extends PureComponent<Props, State> {
isAdmin,
isAnnouncementOnly,
isSubscribed,
stream,
} = this.props;

if (!isSubscribed) {
Expand All @@ -370,6 +392,7 @@ class ComposeBox extends PureComponent<Props, State> {

return (
<View style={this.styles.wrapper}>
<MentionWarnings narrow={narrow} stream={stream} ref={this.mentionWarnings} />
<View style={[this.styles.autocompleteWrapper, { marginBottom: height }]}>
<TopicAutocomplete
isFocused={isTopicFocused}
Expand Down Expand Up @@ -450,4 +473,5 @@ export default connect<SelectorProps, _, _>((state, props) => ({
draft: getDraftForNarrow(state, props.narrow),
lastMessageTopic: getLastMessageTopic(state, props.narrow),
caughtUp: getCaughtUpForNarrow(state, props.narrow),
stream: getStreamInNarrow(state, props.narrow),
}))(withGetText(ComposeBox));
186 changes: 186 additions & 0 deletions src/compose/MentionWarnings.js
Original file line number Diff line number Diff line change
@@ -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<number>,
|};

type SelectorProps = {|
auth: Auth,
usersById: Map<number, UserOrBot>,
|};

type Props = $ReadOnly<{|
narrow: Narrow,
stream: Subscription | {| ...Stream, in_home_view: boolean |},

dispatch: Dispatch,
...SelectorProps,
|}>;

class MentionWarnings extends PureComponent<Props, State> {
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
// `*<user_group_name>*`, 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(
<MentionedUserNotSubscribed
stream={stream}
user={user}
onDismiss={this.handleMentionWarningDismiss}
key={user.user_id}
/>,
);
}

return (
<AnimatedScaleComponent visible={mentionWarnings.length !== 0}>
{mentionWarnings}
</AnimatedScaleComponent>
);
}
}

// $FlowFixMe. TODO: Use a type checked connect call.
export default connect(
state => ({
auth: getAuth(state),
usersById: getActiveUsersById(state),
}),
null,
null,
{ withRef: true },
)(MentionWarnings);
Loading