MM-25832 Prevent permalink view from loading posts multiple times (#4696)

* Prevent permalink view from loading posts multiple times

* Android unhandled exception and initial loader indicator

* Fix permalink tests
This commit is contained in:
Elias Nahum 2020-08-19 19:19:51 -04:00 committed by GitHub
parent 55cfce9b89
commit c095d45330
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 43 additions and 140 deletions

View file

@ -3126,6 +3126,7 @@ export class ClientError extends Error {
intl: { defaultMessage: string; id: string } | { defaultMessage: string; id: string } | { id: string; defaultMessage: string; values: any } | { id: string; defaultMessage: string };
server_error_id: any;
status_code: any;
details: Error;
constructor(baseUrl: string, data: any) {
super(data.message + ': ' + cleanUrlForLogging(baseUrl, data.url));
@ -3134,6 +3135,7 @@ export class ClientError extends Error {
this.intl = data.intl;
this.server_error_id = data.server_error_id;
this.status_code = data.status_code;
this.details = data.details;
// Ensure message is treated as a property of this class when object spreading. Without this,
// copying the object by using `{...error}` would not include the message.

View file

@ -15,27 +15,24 @@ import MaterialIcon from 'react-native-vector-icons/MaterialIcons';
import AwesomeIcon from 'react-native-vector-icons/FontAwesome';
import {Navigation} from 'react-native-navigation';
import {General} from '@mm-redux/constants';
import EventEmitter from '@mm-redux/utils/event_emitter';
import {getLastPostIndex} from '@mm-redux/utils/post_list';
import FormattedText from 'app/components/formatted_text';
import Loading from 'app/components/loading';
import PostList from 'app/components/post_list';
import PostListRetry from 'app/components/post_list_retry';
import SafeAreaView from 'app/components/safe_area_view';
import {marginHorizontal as margin} from 'app/components/safe_area_view/iphone_x_spacing';
import {preventDoubleTap} from 'app/utils/tap';
import {changeOpacity, makeStyleSheetFromTheme} from 'app/utils/theme';
import {
resetToChannel,
goToScreen,
dismissModal,
dismissAllModals,
popToRoot,
} from 'app/actions/navigation';
} from '@actions/navigation';
import FormattedText from '@components/formatted_text';
import Loading from '@components/loading';
import PostList from '@components/post_list';
import PostListRetry from '@components/post_list_retry';
import SafeAreaView from '@components/safe_area_view';
import {marginHorizontal as margin} from '@components/safe_area_view/iphone_x_spacing';
import {General} from '@mm-redux/constants';
import EventEmitter from '@mm-redux/utils/event_emitter';
import {getLastPostIndex} from '@mm-redux/utils/post_list';
import {preventDoubleTap} from '@utils/tap';
import {changeOpacity, makeStyleSheetFromTheme} from '@utils/theme';
Animatable.initializeRegistryWithDefinitions({
growOut: {
@ -76,7 +73,6 @@ export default class Permalink extends PureComponent {
isPermalink: PropTypes.bool,
myMembers: PropTypes.object.isRequired,
onClose: PropTypes.func,
onPress: PropTypes.func,
postIds: PropTypes.array,
theme: PropTypes.object.isRequired,
isLandscape: PropTypes.bool.isRequired,
@ -84,7 +80,6 @@ export default class Permalink extends PureComponent {
};
static defaultProps = {
onPress: () => true,
postIds: [],
};
@ -92,50 +87,10 @@ export default class Permalink extends PureComponent {
intl: intlShape.isRequired,
};
static getDerivedStateFromProps(nextProps, prevState) {
const newState = {};
if (nextProps.focusedPostId !== prevState.focusedPostIdState) {
newState.focusedPostIdState = nextProps.focusedPostId;
}
if (nextProps.channelId && nextProps.channelId !== prevState.channelIdState) {
newState.channelIdState = nextProps.channelId;
}
if (nextProps.channelName && nextProps.channelName !== prevState.channelNameState) {
newState.channelNameState = nextProps.channelName;
}
if (nextProps.postIds && nextProps.postIds.length > 0 && nextProps.postIds !== prevState.postIdsState) {
newState.postIdsState = nextProps.postIds;
}
if (nextProps.focusedPostId !== prevState.focusedPostIdState) {
let loading = true;
if (nextProps.postIds && nextProps.postIds.length >= 10) {
loading = false;
}
newState.loading = loading;
}
if (Object.keys(newState).length === 0) {
return null;
}
return newState;
}
constructor(props) {
super(props);
const {
postIds,
channelId,
channelName,
focusedPostId,
error,
} = props;
const {error, postIds} = props;
let loading = true;
if (postIds && postIds.length >= 10) {
@ -143,14 +98,10 @@ export default class Permalink extends PureComponent {
}
this.state = {
title: channelName,
title: '',
loading,
error: error || '',
retry: false,
channelIdState: channelId,
channelNameState: channelName,
focusedPostIdState: focusedPostId,
postIdsState: postIds,
};
}
@ -159,14 +110,15 @@ export default class Permalink extends PureComponent {
this.mounted = true;
if (this.state.loading) {
this.loadPosts(this.props);
if (this.state.loading && this.props.focusedPostId) {
this.initialLoad = true;
this.loadPosts();
}
}
componentDidUpdate() {
if (this.state.loading) {
this.loadPosts(this.props);
if (this.state.loading && this.props.focusedPostId && !this.initialLoad) {
this.loadPosts();
}
}
@ -225,23 +177,17 @@ export default class Permalink extends PureComponent {
};
handlePress = () => {
const {channelIdState} = this.state;
if (this.viewRef) {
this.viewRef.growOut().then(() => {
this.jumpToChannel(channelIdState);
this.jumpToChannel(this.props.channelId);
});
}
};
jumpToChannel = async (channelId) => {
if (channelId) {
const {actions, channelTeamId, currentTeamId, onClose} = this.props;
const currentChannelId = this.props.channelId;
const {
handleSelectChannel,
handleTeamChange,
} = actions;
const {actions, channelId: currentChannelId, channelTeamId, currentTeamId, onClose} = this.props;
const {handleSelectChannel, handleTeamChange} = actions;
actions.selectPost('');
@ -269,16 +215,22 @@ export default class Permalink extends PureComponent {
}
};
loadPosts = async (props) => {
loadPosts = async () => {
const {intl} = this.context;
const {actions, channelId, currentUserId, focusedPostId, isPermalink, postIds} = props;
const {actions, channelId, currentUserId, focusedPostId, isPermalink, postIds} = this.props;
const {formatMessage} = intl;
let focusChannelId = channelId;
if (this.mounted && !this.initialLoad) {
this.setState({loading: false});
}
if (focusedPostId) {
const post = await actions.getPostThread(focusedPostId, false);
if (post.error && (!postIds || !postIds.length)) {
if (this.mounted && isPermalink && post.error.message.toLowerCase() !== 'network request failed') {
const error = post.error.message.toLowerCase() === 'network request failed';
const connectionError = post.error.details?.message?.toLowerCase() === 'could not connect to the server.';
if (this.mounted && isPermalink && !error && !connectionError) {
this.setState({
error: formatMessage({
id: 'permalink.error.access',
@ -290,7 +242,7 @@ export default class Permalink extends PureComponent {
}),
});
} else if (this.mounted) {
this.setState({error: post.error.message, retry: true});
this.setState({error: post.error.message, retry: true, loading: false});
}
return;
@ -309,7 +261,8 @@ export default class Permalink extends PureComponent {
await actions.getPostsAround(focusChannelId, focusedPostId, 10);
if (this.mounted) {
if (this.initialLoad) {
this.initialLoad = false;
this.setState({loading: false});
}
}
@ -317,8 +270,8 @@ export default class Permalink extends PureComponent {
retry = () => {
if (this.mounted) {
this.initialLoad = false;
this.setState({loading: true, error: null, retry: false});
this.loadPosts(this.props);
}
};
@ -340,19 +293,8 @@ export default class Permalink extends PureComponent {
};
render() {
const {
currentUserId,
focusedPostId,
theme,
isLandscape,
} = this.props;
const {
error,
retry,
loading,
postIdsState,
title,
} = this.state;
const {channelName, currentUserId, focusedPostId, isLandscape, postIds, theme} = this.props;
const {error, loading, retry, title} = this.state;
const style = getStyleSheet(theme);
let postList;
@ -384,8 +326,8 @@ export default class Permalink extends PureComponent {
onHashtagPress={this.handleHashtagPress}
onPermalinkPress={this.handlePermalinkPress}
onPostPress={this.goToThread}
postIds={postIdsState}
lastPostIndex={Platform.OS === 'android' ? getLastPostIndex(postIdsState) : -1}
postIds={postIds}
lastPostIndex={Platform.OS === 'android' ? getLastPostIndex(postIds || []) : -1}
currentUserId={currentUserId}
lastViewedAt={0}
highlightPinnedOrFlagged={false}
@ -430,7 +372,7 @@ export default class Permalink extends PureComponent {
style={style.title}
>
{this.archivedIcon()}
{title}
{title || channelName}
</Text>
</View>
</View>

View file

@ -34,7 +34,6 @@ describe('Permalink', () => {
isPermalink: true,
myMembers: {},
onClose: jest.fn(),
onPress: jest.fn(),
postIds: ['post_id_1', 'focused_post_id', 'post_id_3'],
theme: Preferences.THEMES.default,
componentId: 'component-id',
@ -62,8 +61,7 @@ describe('Permalink', () => {
wrapper.instance().loadPosts = jest.fn();
wrapper.instance().retry();
expect(wrapper.instance().loadPosts).toHaveBeenCalledTimes(2);
expect(wrapper.instance().loadPosts).toBeCalledWith(baseProps);
expect(wrapper.instance().loadPosts).toHaveBeenCalledTimes(1);
});
test('should call handleClose on onNavigatorEvent(backPress)', () => {
@ -76,43 +74,4 @@ describe('Permalink', () => {
wrapper.instance().navigationButtonPressed({buttonId: 'backPress'});
expect(wrapper.instance().handleClose).toHaveBeenCalledTimes(1);
});
test('should match state', () => {
const wrapper = shallow(
<Permalink {...baseProps}/>,
{context: {intl: {formatMessage: jest.fn()}}},
);
expect(wrapper.state('channelIdState')).toEqual(baseProps.channelId);
expect(wrapper.state('channelNameState')).toEqual(baseProps.channelName);
expect(wrapper.state('focusedPostIdState')).toEqual(baseProps.focusedPostId);
expect(wrapper.state('postIdsState')).toEqual(baseProps.postIds);
wrapper.setProps({channelId: ''});
expect(wrapper.state('channelIdState')).toEqual(baseProps.channelId);
wrapper.setProps({channelId: null});
expect(wrapper.state('channelIdState')).toEqual(baseProps.channelId);
wrapper.setProps({channelId: 'new_channel_id'});
expect(wrapper.state('channelIdState')).toEqual('new_channel_id');
wrapper.setProps({channelName: ''});
expect(wrapper.state('channelNameState')).toEqual(baseProps.channelName);
wrapper.setProps({channelName: null});
expect(wrapper.state('channelNameState')).toEqual(baseProps.channelName);
wrapper.setProps({channelName: 'new_channel_name'});
expect(wrapper.state('channelNameState')).toEqual('new_channel_name');
wrapper.setProps({focusedPostId: 'new_focused_post_id'});
expect(wrapper.state('focusedPostIdState')).toEqual('new_focused_post_id');
wrapper.setProps({postIds: []});
expect(wrapper.state('postIdsState')).toEqual(baseProps.postIds);
wrapper.setProps({postIds: ['post_id_1', 'focused_post_id']});
expect(wrapper.state('postIdsState')).toEqual(['post_id_1', 'focused_post_id']);
wrapper.setProps({postIds: baseProps.postIds, focusedPostId: baseProps.focusedPostId});
expect(wrapper.state('loading')).toEqual(true);
wrapper.setProps({postIds: ['1', '2', '3', '4', '5', '6', '7', '8', '9', '10', '11'], focusedPostId: 'new_focused_post_id'});
expect(wrapper.state('loading')).toEqual(false);
});
});