From 0de09209743d9f58bcbb912141db83e31f26f783 Mon Sep 17 00:00:00 2001 From: Saturnino Abril Date: Wed, 20 Feb 2019 01:45:04 +0800 Subject: [PATCH] [MM-13295] Make channel link clickable for user who's not member of the public channel (#2466) * Make channel link clickable for user who's not member of the public channel * update per review * update per comment * remove setChannelDisplayName --- app/components/channel_link/channel_link.js | 54 ++++++++++-- .../channel_link/channel_link.test.js | 87 +++++++++++++++++-- app/components/channel_link/index.js | 36 ++++++-- app/components/markdown/markdown.js | 2 + app/components/post_body/post_body.js | 1 + 5 files changed, 157 insertions(+), 23 deletions(-) diff --git a/app/components/channel_link/channel_link.js b/app/components/channel_link/channel_link.js index 839c0330b..0c3ef67f1 100644 --- a/app/components/channel_link/channel_link.js +++ b/app/components/channel_link/channel_link.js @@ -4,21 +4,27 @@ import React from 'react'; import PropTypes from 'prop-types'; import {Text} from 'react-native'; - -import {getChannelFromChannelName} from './channel_link_utils'; +import {intlShape} from 'react-intl'; import CustomPropTypes from 'app/constants/custom_prop_types'; +import {t} from 'app/utils/i18n'; +import {alertErrorWithFallback} from 'app/utils/general'; + +import {getChannelFromChannelName} from './channel_link_utils'; export default class ChannelLink extends React.PureComponent { static propTypes = { channelName: PropTypes.string.isRequired, + channelMentions: PropTypes.object, + currentTeamId: PropTypes.string.isRequired, + currentUserId: PropTypes.string.isRequired, linkStyle: CustomPropTypes.Style, onChannelLinkPress: PropTypes.func, textStyle: CustomPropTypes.Style, channelsByName: PropTypes.object.isRequired, actions: PropTypes.shape({ handleSelectChannel: PropTypes.func.isRequired, - setChannelDisplayName: PropTypes.func.isRequired, + joinChannel: PropTypes.func.isRequired, }).isRequired, }; @@ -30,6 +36,10 @@ export default class ChannelLink extends React.PureComponent { }; } + static contextTypes = { + intl: intlShape.isRequired, + }; + static getDerivedStateFromProps(nextProps, prevState) { const nextChannel = getChannelFromChannelName(nextProps.channelName, nextProps.channelsByName); if (nextChannel !== prevState.channel) { @@ -39,12 +49,35 @@ export default class ChannelLink extends React.PureComponent { return null; } - handlePress = () => { - this.props.actions.setChannelDisplayName(this.state.channel.display_name); - this.props.actions.handleSelectChannel(this.state.channel.id); + handlePress = async () => { + let {channel} = this.state; - if (this.props.onChannelLinkPress) { - this.props.onChannelLinkPress(this.state.channel); + if (!channel.id && channel.display_name) { + const { + actions, + channelName, + currentTeamId, + currentUserId, + } = this.props; + + const result = await actions.joinChannel(currentUserId, currentTeamId, null, channelName); + if (result.error || !result.data || !result.data.channel) { + const joinFailedMessage = { + id: t('mobile.join_channel.error'), + defaultMessage: "We couldn't join the channel {displayName}. Please check your connection and try again.", + }; + alertErrorWithFallback(this.context.intl, result.error || {}, joinFailedMessage, channel.display_name); + } else if (result?.data?.channel) { + channel = result.data.channel; + } + } + + if (channel.id) { + this.props.actions.handleSelectChannel(channel.id); + + if (this.props.onChannelLinkPress) { + this.props.onChannelLinkPress(channel); + } } } @@ -55,7 +88,10 @@ export default class ChannelLink extends React.PureComponent { return {`~${this.props.channelName}`}; } - const suffix = this.props.channelName.substring(channel.name.length); + let suffix; + if (channel.name) { + suffix = this.props.channelName.substring(channel.name.length); + } return ( diff --git a/app/components/channel_link/channel_link.test.js b/app/components/channel_link/channel_link.test.js index 7d1732646..611b1a120 100644 --- a/app/components/channel_link/channel_link.test.js +++ b/app/components/channel_link/channel_link.test.js @@ -5,28 +5,40 @@ import React from 'react'; import {shallow} from 'enzyme'; import {Text} from 'react-native'; +import {alertErrorWithFallback} from 'app/utils/general'; + import ChannelLink from './channel_link'; +jest.mock('react-intl'); + +jest.mock('app/utils/general', () => ({ + alertErrorWithFallback: jest.fn(), +})); + describe('ChannelLink', () => { + const formatMessage = jest.fn(); const channelsByName = { - firstChannel: {id: 'channel_id_1', name: 'firstChannel', display_name: 'First Channel'}, - secondChannel: {id: 'channel_id_2', name: 'secondChannel', display_name: 'Second Channel'}, + firstChannel: {id: 'channel_id_1', name: 'firstChannel', display_name: 'First Channel', team_id: 'current_team_id'}, + secondChannel: {id: 'channel_id_2', name: 'secondChannel', display_name: 'Second Channel', team_id: 'current_team_id'}, }; const baseProps = { channelName: 'firstChannel', + currentTeamId: 'current_team_id', + currentUserId: 'current_user_id', linkStyle: {color: '#2389d7'}, onChannelLinkPress: jest.fn(), textStyle: {color: '#3d3c40', fontSize: 15, lineHeight: 20}, channelsByName, actions: { handleSelectChannel: jest.fn(), - setChannelDisplayName: jest.fn(), + joinChannel: jest.fn(), }, }; test('should match snapshot', () => { const wrapper = shallow( - + , + {context: {intl: {formatMessage}}}, ); expect(wrapper.getElement()).toMatchSnapshot(); @@ -59,17 +71,76 @@ describe('ChannelLink', () => { test('should call props.actions and onChannelLinkPress on handlePress', () => { const wrapper = shallow( - + , + {context: {intl: {formatMessage}}}, ); const channel = channelsByName.firstChannel; - wrapper.setState({channel}); wrapper.instance().handlePress(); expect(baseProps.actions.handleSelectChannel).toHaveBeenCalledTimes(1); expect(baseProps.actions.handleSelectChannel).toBeCalledWith(channel.id); - expect(baseProps.actions.setChannelDisplayName).toHaveBeenCalledTimes(1); - expect(baseProps.actions.setChannelDisplayName).toBeCalledWith(channel.display_name); expect(baseProps.onChannelLinkPress).toHaveBeenCalledTimes(1); expect(baseProps.onChannelLinkPress).toBeCalledWith(channel); + + expect(baseProps.actions.joinChannel).not.toBeCalled(); + }); + + test('should call props.actions.joinChannel on handlePress when user is not member of such channel', async () => { + const newChannelName = 'thirdChannel'; + const thirdChannel = {id: 'channel_id_3', name: 'thirdChannel', display_name: 'thirdChannel', team_id: 'current_team_id'}; + const error = {message: 'Failed to join a channel'}; + const joinChannel = jest.fn(). + mockReturnValueOnce({data: {channel: thirdChannel}}). + mockReturnValueOnce({}). + mockReturnValueOnce({data: {}}). + mockReturnValueOnce({error}); + const channelMentions = {thirdChannel: {display_name: 'thirdChannel'}}; + const newChannelsByName = Object.assign({}, channelMentions, channelsByName); + const newProps = { + ...baseProps, + channelsByName: newChannelsByName, + channelName: newChannelName, + actions: {...baseProps.actions, joinChannel}, + }; + const intl = {formatMessage}; + const joinFailedMessage = { + id: 'mobile.join_channel.error', + defaultMessage: 'We couldn\'t join the channel {displayName}. Please check your connection and try again.', + }; + const wrapper = shallow( + , + {context: {intl}}, + ); + + await wrapper.instance().handlePress(); + expect(newProps.actions.joinChannel).toHaveBeenCalledTimes(1); + expect(newProps.actions.joinChannel).toBeCalledWith('current_user_id', 'current_team_id', null, newChannelName); + expect(alertErrorWithFallback).not.toBeCalled(); + expect(newProps.actions.handleSelectChannel).toHaveBeenCalledTimes(1); + expect(newProps.actions.handleSelectChannel).toHaveBeenLastCalledWith(thirdChannel.id); + expect(newProps.onChannelLinkPress).toHaveBeenCalledTimes(1); + expect(newProps.onChannelLinkPress).toHaveBeenLastCalledWith(thirdChannel); + + // should have called alertErrorWithFallback on error when joining a channel + await wrapper.instance().handlePress(); + expect(newProps.actions.joinChannel).toHaveBeenCalledTimes(2); + expect(alertErrorWithFallback).toHaveBeenCalledTimes(1); + expect(alertErrorWithFallback).toHaveBeenLastCalledWith(intl, {}, joinFailedMessage, thirdChannel.display_name); + expect(newProps.actions.handleSelectChannel).toHaveBeenCalledTimes(1); + expect(newProps.onChannelLinkPress).toHaveBeenCalledTimes(1); + + await wrapper.instance().handlePress(); + expect(newProps.actions.joinChannel).toHaveBeenCalledTimes(3); + expect(alertErrorWithFallback).toHaveBeenCalledTimes(2); + expect(alertErrorWithFallback).toHaveBeenLastCalledWith(intl, {}, joinFailedMessage, thirdChannel.display_name); + expect(newProps.actions.handleSelectChannel).toHaveBeenCalledTimes(1); + expect(newProps.onChannelLinkPress).toHaveBeenCalledTimes(1); + + await wrapper.instance().handlePress(); + expect(newProps.actions.joinChannel).toHaveBeenCalledTimes(4); + expect(alertErrorWithFallback).toHaveBeenCalledTimes(3); + expect(alertErrorWithFallback).toHaveBeenLastCalledWith(intl, error, joinFailedMessage, thirdChannel.display_name); + expect(newProps.actions.handleSelectChannel).toHaveBeenCalledTimes(1); + expect(newProps.onChannelLinkPress).toHaveBeenCalledTimes(1); }); }); diff --git a/app/components/channel_link/index.js b/app/components/channel_link/index.js index ac9f385b3..6b2ed9edd 100644 --- a/app/components/channel_link/index.js +++ b/app/components/channel_link/index.js @@ -3,16 +3,40 @@ import {connect} from 'react-redux'; import {bindActionCreators} from 'redux'; +import {createSelector} from 'reselect'; +import {joinChannel} from 'mattermost-redux/actions/channels'; import {getChannelsNameMapInCurrentTeam} from 'mattermost-redux/selectors/entities/channels'; +import {getCurrentTeamId} from 'mattermost-redux/selectors/entities/teams'; +import {getCurrentUserId} from 'mattermost-redux/selectors/entities/users'; -import {handleSelectChannel, setChannelDisplayName} from 'app/actions/views/channel'; +import {handleSelectChannel} from 'app/actions/views/channel'; import ChannelLink from './channel_link'; -function mapStateToProps(state) { - return { - channelsByName: getChannelsNameMapInCurrentTeam(state), +function makeGetChannelNamesMap() { + return createSelector( + getChannelsNameMapInCurrentTeam, + (state, props) => props && props.channelMentions, + (channelsNameMap, channelMentions) => { + if (channelMentions) { + return Object.assign({}, channelMentions, channelsNameMap); + } + + return channelsNameMap; + } + ); +} + +function makeMapStateToProps() { + const getChannelNamesMap = makeGetChannelNamesMap(); + + return function mapStateToProps(state, ownProps) { + return { + channelsByName: getChannelNamesMap(state, ownProps), + currentTeamId: getCurrentTeamId(state), + currentUserId: getCurrentUserId(state), + }; }; } @@ -20,9 +44,9 @@ function mapDispatchToProps(dispatch) { return { actions: bindActionCreators({ handleSelectChannel, - setChannelDisplayName, + joinChannel, }, dispatch), }; } -export default connect(mapStateToProps, mapDispatchToProps)(ChannelLink); +export default connect(makeMapStateToProps, mapDispatchToProps)(ChannelLink); diff --git a/app/components/markdown/markdown.js b/app/components/markdown/markdown.js index d14da9d2f..50a523473 100644 --- a/app/components/markdown/markdown.js +++ b/app/components/markdown/markdown.js @@ -42,6 +42,7 @@ export default class Markdown extends PureComponent { autolinkedUrlSchemes: PropTypes.array.isRequired, baseTextStyle: CustomPropTypes.Style, blockStyles: PropTypes.object, + channelMentions: PropTypes.object, imageMetadata: PropTypes.object, isEdited: PropTypes.bool, isReplyPost: PropTypes.bool, @@ -222,6 +223,7 @@ export default class Markdown extends PureComponent { textStyle={this.computeTextStyle(this.props.baseTextStyle, context)} onChannelLinkPress={this.props.onChannelLinkPress} channelName={channelName} + channelMentions={this.props.channelMentions} /> ); }; diff --git a/app/components/post_body/post_body.js b/app/components/post_body/post_body.js index 2590d223a..0f9494668 100644 --- a/app/components/post_body/post_body.js +++ b/app/components/post_body/post_body.js @@ -379,6 +379,7 @@ export default class PostBody extends PureComponent {