Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

Commit

Permalink
Preevnt loading auto load of posts when postList does not exist
Browse files Browse the repository at this point in the history
prevent loading older posts when a callback is in place
Fix range
Sync posts only if postListIds exist
Sync posts only if lastPostTimestamp exists
  • Loading branch information
sudheerDev committed Jul 8, 2019
1 parent 1830d05 commit ff7bb45
Show file tree
Hide file tree
Showing 8 changed files with 246 additions and 169 deletions.
24 changes: 21 additions & 3 deletions actions/views/channel.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,13 +132,19 @@ export function autocompleteUsersInChannel(prefix, channelId) {

export function loadUnreads(channelId) {
return async (dispatch) => {
const {data} = await dispatch(PostActions.getPostsUnread(channelId));
const {data, error} = await dispatch(PostActions.getPostsUnread(channelId));
if (data) {
dispatch({
type: ActionTypes.INCREASE_POST_VISIBILITY,
data: channelId,
amount: data.order.length,
});
} else {
return {
error,
atLatestMessage: true,
atOldestmessage: true,
};
}

return {
Expand All @@ -150,13 +156,19 @@ export function loadUnreads(channelId) {

export function loadPostsAround(channelId, focusedPostId) {
return async (dispatch) => {
const {data} = await dispatch(PostActions.getPostsAround(channelId, focusedPostId, Posts.POST_CHUNK_SIZE / 2));
const {data, error} = await dispatch(PostActions.getPostsAround(channelId, focusedPostId, Posts.POST_CHUNK_SIZE / 2));
if (data) {
dispatch({
type: ActionTypes.INCREASE_POST_VISIBILITY,
data: channelId,
amount: data.order.length,
});
} else {
return {
error,
atLatestMessage: true,
atOldestmessage: true,
};
}
return {
atLatestMessage: data.next_post_id === '',
Expand All @@ -168,14 +180,20 @@ export function loadPostsAround(channelId, focusedPostId) {
export function loadLatestPosts(channelId) {
return async (dispatch) => {
const time = Date.now();
const {data} = await dispatch(PostActions.getPosts(channelId, 0, Posts.POST_CHUNK_SIZE / 2));
const {data, error} = await dispatch(PostActions.getPosts(channelId, 0, Posts.POST_CHUNK_SIZE / 2));

if (data) {
dispatch({
type: ActionTypes.RECEIVED_POSTS_FOR_CHANNEL_AT_TIME,
channelId,
time,
});
} else {
return {
error,
atLatestMessage: true,
atOldestmessage: true,
};
}

return {
Expand Down
32 changes: 32 additions & 0 deletions actions/views/channel.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,38 @@ describe('channel view actions', () => {
});
});

describe('loadUnreads', () => {
test('when there are no posts after and before the response', async () => {
const posts = {posts: {}, order: [], next_post_id: '', prev_post_id: ''};

PostActions.getPostsUnread.mockReturnValue(() => ({data: posts}));

const result = await store.dispatch(Actions.loadUnreads('channel', 'post'));

expect(result).toEqual({atLatestMessage: true, atOldestmessage: true});
expect(PostActions.getPostsUnread).toHaveBeenCalledWith('channel');
});

test('when there are posts before and after the response', async () => {
const posts = {
posts: {},
order: [
...new Array(Posts.POST_CHUNK_SIZE / 2), // after
'post',
...new Array(Posts.POST_CHUNK_SIZE / 2), // before
],
next_post_id: 'test',
prev_post_id: 'test',
};

PostActions.getPostsUnread.mockReturnValue(() => ({data: posts}));

const result = await store.dispatch(Actions.loadUnreads('channel', 'post'));
expect(result).toEqual({atLatestMessage: false, atOldestmessage: false});
expect(PostActions.getPostsUnread).toHaveBeenCalledWith('channel');
});
});

describe('loadPostsAround', () => {
test('should call getPostsAround and return the results', async () => {
const posts = {posts: {}, order: [], next_post_id: '', prev_post_id: ''};
Expand Down
55 changes: 37 additions & 18 deletions components/post_view/post_list/post_list.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ import PropTypes from 'prop-types';

import LoadingScreen from 'components/loading_screen.jsx';
import {PostRequestTypes} from 'utils/constants.jsx';
import {getOldestPostId, getLatestPostId} from 'utils/post_utils.jsx';

import VirtPostList from './post_list_virtualized.jsx';

const MAX_NUMBER_OF_AUTO_RETRIES = 3;
const MAX_EXTRA_PAGES_LOADED = 10;
export const MAX_EXTRA_PAGES_LOADED = 10;

export default class PostList extends React.PureComponent {
static propTypes = {
Expand Down Expand Up @@ -100,6 +101,7 @@ export default class PostList extends React.PureComponent {
};

this.autoRetriesCount = 0;
this.loadingMorePosts = null;
this.actionsForPostList = {
loadOlderPosts: this.getPostsBefore,
loadNewerPosts: this.getPostsAfter,
Expand Down Expand Up @@ -131,7 +133,7 @@ export default class PostList extends React.PureComponent {
await this.loadPermalinkPosts(channelId);
} else if (this.props.isFirstLoad) {
await this.loadUnreadPosts(channelId);
} else if (this.props.postListIds) {
} else if (this.props.latestPostTimeStamp) {
await this.props.actions.syncPostsInChannel(channelId, this.props.latestPostTimeStamp);
} else {
await this.loadLatestPosts(channelId);
Expand Down Expand Up @@ -239,22 +241,20 @@ export default class PostList extends React.PureComponent {
});
}

canLoadMorePosts = async () => {
if (this.props.focusedPostId) {
return;
}
getOldestVisiblePostId = () => {
return getOldestPostId(this.props.postListIds);
}

if (this.state.loadingFirstSetOfPosts) {
// Should already be loading posts
return;
}
getLatestVisiblePostId = () => {
return getLatestPostId(this.props.postListIds);
}

if (this.state.olderPosts.allLoaded) {
// Screen is full
canLoadMorePosts = async () => {
if (!this.props.postListIds) {
return;
}

if (this.state.olderPosts.loading) {
if (this.state.olderPosts.loading || this.state.newerPosts.loading) {
return;
}

Expand All @@ -265,18 +265,37 @@ export default class PostList extends React.PureComponent {
return;
}

await this.getPostsBefore();
if (this.loadingMorePosts) {
return;
}

//canLoadMorePosts can be called few times in a setState cycle so having a instance
//variable to prevent from calling getPosts call when one call is in place
this.loadingMorePosts = true;

if (!this.state.olderPosts.allLoaded) {
const oldestPostId = this.getOldestVisiblePostId();
await this.getPostsBefore(oldestPostId);
} else if (!this.state.newerPosts.allLoaded) {
// if all olderPosts are loaded load new ones
const latestPostId = this.getLatestVisiblePostId();
await this.getPostsAfter(latestPostId);
}

this.loadingMorePosts = false;
this.extraPagesLoaded += 1;
}

getPostsBefore = (oldestPostId) => {
getPostsBefore = async () => {
const oldestPostId = this.getOldestVisiblePostId();
this.setLoadingPosts('olderPosts');
return this.callLoadPosts(this.props.channelId, oldestPostId, PostRequestTypes.BEFORE_ID);
await this.callLoadPosts(this.props.channelId, oldestPostId, PostRequestTypes.BEFORE_ID);
}

getPostsAfter = (newestMessageId) => {
getPostsAfter = async () => {
const latestPostId = this.getLatestVisiblePostId();
this.setLoadingPosts('newerPosts');
return this.callLoadPosts(this.props.channelId, newestMessageId, PostRequestTypes.AFTER_ID);
await this.callLoadPosts(this.props.channelId, latestPostId, PostRequestTypes.AFTER_ID);
}

render() {
Expand Down
102 changes: 93 additions & 9 deletions components/post_view/post_list/post_list.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {shallow} from 'enzyme';

import {PostRequestTypes} from 'utils/constants.jsx';

import PostList from './post_list.jsx';
import PostList, {MAX_EXTRA_PAGES_LOADED} from './post_list.jsx';
import VirtPostList from './post_list_virtualized.jsx';

const actionsProp = {
Expand Down Expand Up @@ -76,15 +76,15 @@ describe('components/post_view/post_list', () => {
<PostList {...{...baseProps, postListIds: postIds}}/>
);

wrapper.find(VirtPostList).prop('actions').loadOlderPosts('1234');
wrapper.find(VirtPostList).prop('actions').loadOlderPosts();
expect(wrapper.state('olderPosts').loading).toEqual(true);
expect(actionsProp.loadPosts).toHaveBeenCalledWith({channelId: baseProps.channelId, postId: '1234', type: PostRequestTypes.BEFORE_ID});
expect(actionsProp.loadPosts).toHaveBeenCalledWith({channelId: baseProps.channelId, postId: postIds[postIds.length - 1], type: PostRequestTypes.BEFORE_ID});
await wrapper.instance().callLoadPosts();
expect(wrapper.state('olderPosts')).toEqual({allLoaded: true, loading: false});

wrapper.find(VirtPostList).prop('actions').loadNewerPosts('1234');
wrapper.find(VirtPostList).prop('actions').loadNewerPosts();
expect(wrapper.state('newerPosts').loading).toEqual(true);
expect(actionsProp.loadPosts).toHaveBeenCalledWith({channelId: baseProps.channelId, postId: '1234', type: PostRequestTypes.AFTER_ID});
expect(actionsProp.loadPosts).toHaveBeenCalledWith({channelId: baseProps.channelId, postId: postIds[0], type: PostRequestTypes.AFTER_ID});
await wrapper.instance().callLoadPosts();
expect(wrapper.state('newerPosts')).toEqual({allLoaded: true, loading: false});
});
Expand All @@ -102,9 +102,8 @@ describe('components/post_view/post_list', () => {
});

it('Should call for loadLatestPosts', async () => {
const noPostList = undefined;
const wrapper = shallow(
<PostList {...{...baseProps, postListIds: noPostList, isFirstLoad: false}}/>
<PostList {...{...baseProps, postListIds: [], isFirstLoad: false}}/>
);

expect(actionsProp.loadLatestPosts).toHaveBeenCalledWith(baseProps.channelId);
Expand All @@ -116,8 +115,93 @@ describe('components/post_view/post_list', () => {
describe('getPostsSince', () => {
test('should call getPostsSince on channel switch', () => {
const postIds = createFakePosIds(2);
shallow(<PostList {...{...baseProps, isFirstLoad: false, postListIds: postIds}}/>);
expect(actionsProp.syncPostsInChannel).toHaveBeenCalledWith(baseProps.channelId, baseProps.latestPostTimeStamp);
shallow(<PostList {...{...baseProps, isFirstLoad: false, postListIds: postIds, latestPostTimeStamp: 1234}}/>);
expect(actionsProp.syncPostsInChannel).toHaveBeenCalledWith(baseProps.channelId, 1234);
});
});

describe('canLoadMorePosts', () => {
test('Should not call loadPosts if postListIds is empty', async () => {
const wrapper = shallow(<PostList {...{...baseProps, isFirstLoad: false, postListIds: []}}/>);
await wrapper.instance().loadLatestPosts();
wrapper.find(VirtPostList).prop('actions').canLoadMorePosts();
expect(actionsProp.loadPosts).not.toHaveBeenCalled();
});

test('Should not call loadPosts if all olderPosts are loaded or loading', async () => {
const postIds = createFakePosIds(2);
const wrapper = shallow(<PostList {...{...baseProps, isFirstLoad: false, postListIds: postIds}}/>);
await wrapper.instance().loadLatestPosts();
wrapper.setState({olderPosts: {allLoaded: false, loading: true}});
wrapper.find(VirtPostList).prop('actions').canLoadMorePosts();
expect(actionsProp.loadPosts).not.toHaveBeenCalled();
wrapper.setState({olderPosts: {allLoaded: true, loading: false}});
wrapper.find(VirtPostList).prop('actions').canLoadMorePosts();
expect(actionsProp.loadPosts).not.toHaveBeenCalled();
});

test('Should not call loadPosts if there were more than MAX_EXTRA_PAGES_LOADED', async () => {
const postIds = createFakePosIds(2);
const wrapper = shallow(<PostList {...{...baseProps, isFirstLoad: false, postListIds: postIds}}/>);
wrapper.instance().extraPagesLoaded = MAX_EXTRA_PAGES_LOADED + 1;
wrapper.find(VirtPostList).prop('actions').canLoadMorePosts();
await wrapper.instance().loadLatestPosts();
expect(actionsProp.loadPosts).not.toHaveBeenCalled();
});

test('Should not call loadPosts if there were more than MAX_EXTRA_PAGES_LOADED', async () => {
const postIds = createFakePosIds(2);
const wrapper = shallow(<PostList {...{...baseProps, isFirstLoad: false, postListIds: postIds}}/>);
wrapper.instance().loadingMorePosts = true;
wrapper.find(VirtPostList).prop('actions').canLoadMorePosts();
await wrapper.instance().loadLatestPosts();
expect(actionsProp.loadPosts).not.toHaveBeenCalled();
});

test('Should call getPostsBefore if not all older posts are loaded', async () => {
const postIds = createFakePosIds(2);
const wrapper = shallow(<PostList {...{...baseProps, isFirstLoad: false, postListIds: postIds}}/>);
wrapper.setState({olderPosts: {allLoaded: false, loading: false}});
wrapper.find(VirtPostList).prop('actions').canLoadMorePosts();
await wrapper.instance().loadLatestPosts();
expect(actionsProp.loadPosts).toHaveBeenCalledWith({channelId: baseProps.channelId, postId: postIds[postIds.length - 1], type: PostRequestTypes.BEFORE_ID});
});

test('Should call getPostsAfter if all older posts are loaded and not newerPosts', async () => {
const postIds = createFakePosIds(2);
const wrapper = shallow(<PostList {...{...baseProps, isFirstLoad: false, postListIds: postIds}}/>);
wrapper.setState({olderPosts: {allLoaded: true, loading: false}});
wrapper.setState({newerPosts: {allLoaded: false, loading: false}});
wrapper.find(VirtPostList).prop('actions').canLoadMorePosts();
await wrapper.instance().loadLatestPosts();
expect(actionsProp.loadPosts).toHaveBeenCalledWith({channelId: baseProps.channelId, postId: postIds[0], type: PostRequestTypes.AFTER_ID});
});
});

describe('Auto retry of load more posts', () => {
test('Should retry loadPosts on failure of loadPosts', async () => {
const postIds = createFakePosIds(2);
const loadPosts = jest.fn().mockImplementation(() => Promise.resolve({moreToLoad: true, error: {}}));
const props = {
...baseProps,
postListIds: postIds,
actions: {
...actionsProp,
loadPosts,
},
};

const wrapper = shallow(
<PostList {...props}/>
);

wrapper.find(VirtPostList).prop('actions').loadOlderPosts();
expect(wrapper.state('olderPosts').loading).toEqual(true);
expect(loadPosts).toHaveBeenCalledTimes(1);
expect(loadPosts).toHaveBeenCalledWith({channelId: baseProps.channelId, postId: postIds[postIds.length - 1], type: PostRequestTypes.BEFORE_ID});
await loadPosts();
expect(wrapper.state('olderPosts')).toEqual({allLoaded: false, loading: false});
expect(loadPosts).toHaveBeenCalledTimes(3);
});
});
});
Loading

0 comments on commit ff7bb45

Please sign in to comment.