Unverified Commit 1945d829 authored by Guillermo Vayá's avatar Guillermo Vayá Committed by GitHub

[MM-20725] Revert threads performance change on 5.18 (#994)

* Revert "MM-18623 - Handle reply-count in getPostThread code path (#933)"

This reverts commit 74018c66.

* Revert "MM-17468 - Fix regression. Maintain reply_count on parent post w… (#928)"

This reverts commit 4a749d7a.

* Revert "MM-17468 - Improving performance of fetching threads (#911)"

This reverts commit ae457c33.

* prevent circle from changing package-lock on install
parent 4159c86e
......@@ -44,6 +44,21 @@
"ms": "^2.1.1"
}
},
"json5": {
"version": "2.1.0",
"resolved": "https://registry.npmjs.org/json5/-/json5-2.1.0.tgz",
"integrity": "sha512-8Mh9h6xViijj36g7Dxi+Y4S6hNGV96vcJZr/SrlHh1LR/pEn/8j/+qIBbs44YKl69Lrfctp4QD+AdWLTMqEZAQ==",
"dev": true,
"requires": {
"minimist": "^1.2.0"
}
},
"minimist": {
"version": "1.2.0",
"resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz",
"integrity": "sha1-o1AIsg9BOD7sH7kU9M1d95omQoQ=",
"dev": true
},
"ms": {
"version": "2.1.2",
"resolved": "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz",
......@@ -6798,6 +6813,14 @@
"micromatch": "^3.1.4",
"minimist": "^1.1.1",
"walker": "~1.0.5"
},
"dependencies": {
"minimist": {
"version": "1.2.0",
"resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz",
"integrity": "sha1-o1AIsg9BOD7sH7kU9M1d95omQoQ=",
"dev": true
}
}
},
"sax": {
......
......@@ -64,34 +64,6 @@ describe('Actions.Posts', () => {
assert.ok(!postsInChannel[channelId], 'postIds in channel do not exist');
});
it('maintain reply_count', async () => {
const channelId = TestHelper.basicChannel.id;
const post = TestHelper.fakePostWithId(channelId);
const post2 = TestHelper.fakePostWithId(channelId);
post2.root_id = post.id;
nock(Client4.getPostsRoute()).
post('').
reply(201, post);
await Actions.createPost(post)(store.dispatch, store.getState);
nock(Client4.getPostsRoute()).
post('').
reply(201, post2);
await Actions.createPost(post2)(store.dispatch, store.getState);
assert.equal(store.getState().entities.posts.posts[post.id].reply_count, 1);
await Actions.deletePost(post2)(store.dispatch, store.getState);
await Actions.removePost(post2)(store.dispatch, store.getState);
assert.equal(store.getState().entities.posts.posts[post.id].reply_count, 0);
nock.cleanAll();
});
it('resetCreatePostRequest', async () => {
const channelId = TestHelper.basicChannel.id;
const post = TestHelper.fakePost(channelId);
......@@ -438,7 +410,7 @@ describe('Actions.Posts', () => {
};
nock(Client4.getPostsRoute()).
get(`/${post.id}/thread?fetchThreads=true`).
get(`/${post.id}/thread`).
reply(200, postList);
await Actions.getPostThread(post.id)(store.dispatch, store.getState);
......@@ -1207,7 +1179,7 @@ describe('Actions.Posts', () => {
postList.posts[post1.id] = post1;
nock(Client4.getPostsRoute()).
get(`/${post1.id}/thread?fetchThreads=true`).
get(`/${post1.id}/thread`).
reply(200, postList);
await Actions.getPostThread(post1.id)(dispatch, getState);
......@@ -1236,7 +1208,7 @@ describe('Actions.Posts', () => {
postList.posts[post1.id] = post1;
nock(Client4.getPostsRoute()).
get(`/${post1.id}/thread?fetchThreads=true`).
get(`/${post1.id}/thread`).
reply(200, postList);
await Actions.getPostThread(post1.id)(dispatch, getState);
......@@ -1671,7 +1643,7 @@ describe('Actions.Posts', () => {
};
nock(Client4.getPostsRoute()).
get(`/${post1.id}/thread?fetchThreads=true`).
get(`/${post1.id}/thread`).
reply(200, threadList);
});
......
......@@ -644,13 +644,13 @@ export function flagPost(postId: string) {
};
}
export function getPostThread(rootId: string, fetchThreads = true) {
export function getPostThread(rootId: string) {
return async (dispatch: DispatchFunc, getState: GetStateFunc) => {
dispatch({type: PostTypes.GET_POST_THREAD_REQUEST}, getState);
let posts;
try {
posts = await Client4.getPostThread(rootId, fetchThreads);
posts = await Client4.getPostThread(rootId);
getProfilesAndStatusesForPosts(posts.posts, dispatch, getState);
} catch (error) {
forceLogoutIfNecessary(error, dispatch, getState);
......@@ -673,12 +673,12 @@ export function getPostThread(rootId: string, fetchThreads = true) {
};
}
export function getPosts(channelId: string, page = 0, perPage = Posts.POST_CHUNK_SIZE, fetchThreads = true) {
export function getPosts(channelId: string, page = 0, perPage = Posts.POST_CHUNK_SIZE) {
return async (dispatch: DispatchFunc, getState: GetStateFunc) => {
let posts;
try {
posts = await Client4.getPosts(channelId, page, perPage, fetchThreads);
posts = await Client4.getPosts(channelId, page, perPage);
getProfilesAndStatusesForPosts(posts.posts, dispatch, getState);
} catch (error) {
forceLogoutIfNecessary(error, dispatch, getState);
......@@ -695,12 +695,12 @@ export function getPosts(channelId: string, page = 0, perPage = Posts.POST_CHUNK
};
}
export function getPostsUnread(channelId: string, fetchThreads = true) {
export function getPostsUnread(channelId: string) {
return async (dispatch: DispatchFunc, getState: GetStateFunc) => {
const userId = getCurrentUserId(getState());
let posts;
try {
posts = await Client4.getPostsUnread(channelId, userId, DEFAULT_LIMIT_BEFORE, DEFAULT_LIMIT_AFTER, fetchThreads);
posts = await Client4.getPostsUnread(channelId, userId);
getProfilesAndStatusesForPosts(posts.posts, dispatch, getState);
} catch (error) {
forceLogoutIfNecessary(error, dispatch, getState);
......@@ -722,11 +722,11 @@ export function getPostsUnread(channelId: string, fetchThreads = true) {
};
}
export function getPostsSince(channelId: string, since: number, fetchThreads = true) {
export function getPostsSince(channelId: string, since: number) {
return async (dispatch: DispatchFunc, getState: GetStateFunc) => {
let posts;
try {
posts = await Client4.getPostsSince(channelId, since, fetchThreads);
posts = await Client4.getPostsSince(channelId, since);
getProfilesAndStatusesForPosts(posts.posts, dispatch, getState);
} catch (error) {
forceLogoutIfNecessary(error, dispatch, getState);
......@@ -746,11 +746,11 @@ export function getPostsSince(channelId: string, since: number, fetchThreads = t
};
}
export function getPostsBefore(channelId: string, postId: string, page = 0, perPage = Posts.POST_CHUNK_SIZE, fetchThreads = true) {
export function getPostsBefore(channelId: string, postId: string, page = 0, perPage = Posts.POST_CHUNK_SIZE) {
return async (dispatch: DispatchFunc, getState: GetStateFunc) => {
let posts;
try {
posts = await Client4.getPostsBefore(channelId, postId, page, perPage, fetchThreads);
posts = await Client4.getPostsBefore(channelId, postId, page, perPage);
getProfilesAndStatusesForPosts(posts.posts, dispatch, getState);
} catch (error) {
forceLogoutIfNecessary(error, dispatch, getState);
......@@ -767,11 +767,11 @@ export function getPostsBefore(channelId: string, postId: string, page = 0, perP
};
}
export function getPostsAfter(channelId: string, postId: string, page = 0, perPage = Posts.POST_CHUNK_SIZE, fetchThreads = true) {
export function getPostsAfter(channelId: string, postId: string, page = 0, perPage = Posts.POST_CHUNK_SIZE) {
return async (dispatch: DispatchFunc, getState: GetStateFunc) => {
let posts;
try {
posts = await Client4.getPostsAfter(channelId, postId, page, perPage, fetchThreads);
posts = await Client4.getPostsAfter(channelId, postId, page, perPage);
getProfilesAndStatusesForPosts(posts.posts, dispatch, getState);
} catch (error) {
forceLogoutIfNecessary(error, dispatch, getState);
......@@ -794,7 +794,7 @@ export type CombinedPostList = {
prev_post_id: string;
}
export function getPostsAround(channelId: string, postId: string, perPage = Posts.POST_CHUNK_SIZE / 2, fetchThreads = true) {
export function getPostsAround(channelId: string, postId: string, perPage = Posts.POST_CHUNK_SIZE / 2) {
return async (dispatch: DispatchFunc, getState: GetStateFunc) => {
let after;
let thread;
......@@ -802,9 +802,9 @@ export function getPostsAround(channelId: string, postId: string, perPage = Post
try {
[after, thread, before] = await Promise.all([
Client4.getPostsAfter(channelId, postId, 0, perPage, fetchThreads),
Client4.getPostThread(postId, fetchThreads),
Client4.getPostsBefore(channelId, postId, 0, perPage, fetchThreads),
Client4.getPostsAfter(channelId, postId, 0, perPage),
Client4.getPostThread(postId),
Client4.getPostsBefore(channelId, postId, 0, perPage),
]);
} catch (error) {
forceLogoutIfNecessary(error, dispatch, getState);
......@@ -841,7 +841,7 @@ export function getPostsAround(channelId: string, postId: string, perPage = Post
// getThreadsForPosts is intended for an array of posts that have been batched
// (see the actions/websocket_actions/handleNewPostEvents function in the webapp)
export function getThreadsForPosts(posts: Array<Post>, fetchThreads = true) {
export function getThreadsForPosts(posts: Array<Post>) {
return (dispatch: DispatchFunc, getState: GetStateFunc) => {
if (!Array.isArray(posts) || !posts.length) {
return {data: true};
......@@ -857,7 +857,7 @@ export function getThreadsForPosts(posts: Array<Post>, fetchThreads = true) {
const rootPost = Selectors.getPost(state, post.root_id);
if (!rootPost) {
promises.push(dispatch(getPostThread(post.root_id, fetchThreads)));
promises.push(dispatch(getPostThread(post.root_id)));
}
});
......@@ -1212,7 +1212,7 @@ function completePostReceive(post: Post, websocketMessageProps: any) {
const rootPost = Selectors.getPost(state, post.root_id);
if (post.root_id && !rootPost) {
dispatch(getPostThread(post.root_id, true));
dispatch(getPostThread(post.root_id));
}
dispatch(lastPostActions(post, websocketMessageProps) as any);
......
......@@ -36,6 +36,7 @@ const PER_PAGE_DEFAULT = 60;
const LOGS_PER_PAGE_DEFAULT = 10000;
export const DEFAULT_LIMIT_BEFORE = 30;
export const DEFAULT_LIMIT_AFTER = 30;
/* eslint-disable no-throw-literal */
export default class Client4 {
......@@ -1606,48 +1607,48 @@ export default class Client4 {
);
};
getPostThread = async (postId: string, fetchThreads = true) => {
getPostThread = async (postId: string) => {
return this.doFetch(
`${this.getPostRoute(postId)}/thread${buildQueryString({fetchThreads})}`,
`${this.getPostRoute(postId)}/thread`,
{method: 'get'}
);
};
getPosts = async (channelId: string, page = 0, perPage = PER_PAGE_DEFAULT, fetchThreads = true) => {
getPosts = async (channelId: string, page = 0, perPage = PER_PAGE_DEFAULT) => {
return this.doFetch(
`${this.getChannelRoute(channelId)}/posts${buildQueryString({page, per_page: perPage, fetchThreads})}`,
`${this.getChannelRoute(channelId)}/posts${buildQueryString({page, per_page: perPage})}`,
{method: 'get'}
);
};
getPostsUnread = async (channelId: string, userId: string, limitAfter = DEFAULT_LIMIT_AFTER, limitBefore = DEFAULT_LIMIT_BEFORE, fetchThreads = true) => {
getPostsUnread = async (channelId: string, userId: string, limitAfter = DEFAULT_LIMIT_AFTER, limitBefore = DEFAULT_LIMIT_BEFORE) => {
return this.doFetch(
`${this.getUserRoute(userId)}/channels/${channelId}/posts/unread${buildQueryString({limit_after: limitAfter, limit_before: limitBefore, fetchThreads})}`,
`${this.getUserRoute(userId)}/channels/${channelId}/posts/unread${buildQueryString({limit_after: limitAfter, limit_before: limitBefore})}`,
{method: 'get'}
);
};
getPostsSince = async (channelId: string, since: number, fetchThreads = true) => {
getPostsSince = async (channelId: string, since: number) => {
return this.doFetch(
`${this.getChannelRoute(channelId)}/posts${buildQueryString({since, fetchThreads})}`,
`${this.getChannelRoute(channelId)}/posts${buildQueryString({since})}`,
{method: 'get'}
);
};
getPostsBefore = async (channelId: string, postId: string, page = 0, perPage = PER_PAGE_DEFAULT, fetchThreads = true) => {
getPostsBefore = async (channelId: string, postId: string, page = 0, perPage = PER_PAGE_DEFAULT) => {
this.trackEvent('api', 'api_posts_get_before', {channel_id: channelId});
return this.doFetch(
`${this.getChannelRoute(channelId)}/posts${buildQueryString({before: postId, page, per_page: perPage, fetchThreads})}`,
`${this.getChannelRoute(channelId)}/posts${buildQueryString({before: postId, page, per_page: perPage})}`,
{method: 'get'}
);
};
getPostsAfter = async (channelId: string, postId: string, page = 0, perPage = PER_PAGE_DEFAULT, fetchThreads = true) => {
getPostsAfter = async (channelId: string, postId: string, page = 0, perPage = PER_PAGE_DEFAULT) => {
this.trackEvent('api', 'api_posts_get_after', {channel_id: channelId});
return this.doFetch(
`${this.getChannelRoute(channelId)}/posts${buildQueryString({after: postId, page, per_page: perPage, fetchThreads})}`,
`${this.getChannelRoute(channelId)}/posts${buildQueryString({after: postId, page, per_page: perPage})}`,
{method: 'get'}
);
};
......
......@@ -69,16 +69,7 @@ export function handlePosts(state: RelationOneToOne<Post, Post> = {}, action: Ge
switch (action.type) {
case PostTypes.RECEIVED_POST:
case PostTypes.RECEIVED_NEW_POST: {
const post = action.data;
const newState = {...state};
if (action.type === PostTypes.RECEIVED_NEW_POST && post.root_id && state[post.root_id] && post.pending_post_id && post.id !== post.pending_post_id) {
const rootPost = state[post.root_id];
newState[post.root_id] = {...rootPost, reply_count: (rootPost.reply_count || 0) + 1};
}
return handlePostReceived(newState, post);
return handlePostReceived({...state}, action.data);
}
case PostTypes.RECEIVED_POSTS: {
......@@ -114,10 +105,6 @@ export function handlePosts(state: RelationOneToOne<Post, Post> = {}, action: Ge
has_reactions: false,
},
};
if (post.root_id && state[post.root_id]) {
const rootPost = state[post.root_id];
nextState[post.root_id] = {...rootPost, reply_count: (rootPost.reply_count || 0) - 1};
}
// Remove any of its comments
for (const otherPost of Object.values(state)) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment