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

fix: calculate pagination stop from custom channel query message limit #2180

Merged
merged 1 commit into from
Nov 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions src/components/Channel/Channel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ import {
DEFAULT_THREAD_PAGE_SIZE,
} from '../../constants/limits';

import { hasMoreMessagesProbably, hasNotMoreMessages } from '../MessageList/utils';
import { hasMoreMessagesProbably } from '../MessageList/utils';
import defaultEmojiData from '../../stream-emoji.json';
import { makeAddNotifications } from './utils';
import { getChannel } from '../../utils/getChannel';
Expand Down Expand Up @@ -490,6 +490,7 @@ const ChannelInner = <
/**
* As the channel state is not normalized we re-fetch the channel data. Thus, we avoid having to search for user references in the channel state.
*/
// FIXME: we should use channelQueryOptions if they are available
await channel.query({
messages: { id_lt: oldestID, limit: DEFAULT_NEXT_CHANNEL_PAGE_SIZE },
watchers: { limit: DEFAULT_NEXT_CHANNEL_PAGE_SIZE },
Expand Down Expand Up @@ -542,7 +543,14 @@ const ChannelInner = <
originalTitle.current = document.title;

if (!errored) {
dispatch({ channel, type: 'initStateFromChannel' });
dispatch({
channel,
hasMore: hasMoreMessagesProbably(
channel.state.messages.length,
channelQueryOptions?.messages?.limit ?? DEFAULT_INITIAL_CHANNEL_PAGE_SIZE,
),
type: 'initStateFromChannel',
});
if (channel.countUnread() > 0) markRead();
// The more complex sync logic is done in Chat
document.addEventListener('visibilitychange', onVisibilityChange);
Expand Down Expand Up @@ -598,7 +606,7 @@ const ChannelInner = <
);

const loadMore = async (limit = DEFAULT_NEXT_CHANNEL_PAGE_SIZE) => {
if (!online.current || !window.navigator.onLine) return 0;
if (!online.current || !window.navigator.onLine || !state.hasMore) return 0;

// prevent duplicate loading events...
const oldestMessage = state?.messages?.[0];
Expand All @@ -607,16 +615,6 @@ const ChannelInner = <
return 0;
}

// initial state loads with up to 25 messages, so if less than 25 no need for additional query
const notHasMore = hasNotMoreMessages(
channel.state.messages.length,
DEFAULT_INITIAL_CHANNEL_PAGE_SIZE,
);
if (notHasMore) {
loadMoreFinished(false, channel.state.messages);
return channel.state.messages.length;
}

dispatch({ loadingMore: true, type: 'setLoadingMore' });

const oldestID = oldestMessage?.id;
Expand Down Expand Up @@ -701,6 +699,7 @@ const ChannelInner = <

const jumpToLatestMessage = async () => {
await channel.state.loadMessageIntoState('latest');
// FIXME: we cannot rely on constant value 25 as the page size can be customized by integrators
const hasMoreOlder = channel.state.messages.length >= 25;
loadMoreFinished(hasMoreOlder, channel.state.messages);
dispatch({
Expand Down Expand Up @@ -913,6 +912,7 @@ const ChannelInner = <
);

const loadMoreThread = async (limit: number = DEFAULT_THREAD_PAGE_SIZE) => {
// FIXME: should prevent loading more, if state.thread.reply_count === channel.state.threads[parentID].length
if (state.threadLoadingMore || !state.thread) return;

dispatch({ type: 'startLoadingThread' });
Expand Down
250 changes: 236 additions & 14 deletions src/components/Channel/__tests__/Channel.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,19 @@ jest.mock('../../Loading', () => ({
LoadingIndicator: jest.fn(() => <div>loading</div>),
}));

const queryChannelWithNewMessages = (newMessages, channel) =>
// generate new channel mock from existing channel with new messages added
getOrCreateChannelApi(
generateChannel({
channel: {
config: channel.getConfig(),
id: channel.id,
type: channel.type,
},
messages: newMessages,
}),
);

const MockAvatar = ({ name }) => (
<div className='avatar' data-testid='custom-avatar'>
{name}
Expand Down Expand Up @@ -294,6 +307,83 @@ describe('Channel', () => {
});
});

it('should set hasMore state to false if the initial channel query returns less messages than the default initial page size', async () => {
const { channel, chatClient } = await initClient();
useMockedApis(chatClient, [queryChannelWithNewMessages([generateMessage()], channel)]);
let hasMore;
await act(() => {
renderComponent({ channel, chatClient }, ({ hasMore: contextHasMore }) => {
hasMore = contextHasMore;
});
});

await waitFor(() => {
expect(hasMore).toBe(false);
});
});

it('should set hasMore state to true if the initial channel query returns count of messages equal to the default initial page size', async () => {
const { channel, chatClient } = await initClient();
useMockedApis(chatClient, [
queryChannelWithNewMessages(Array.from({ length: 25 }, generateMessage), channel),
]);
let hasMore;
await act(() => {
renderComponent({ channel, chatClient }, ({ hasMore: contextHasMore }) => {
hasMore = contextHasMore;
});
});

await waitFor(() => {
expect(hasMore).toBe(true);
});
});

it('should set hasMore state to false if the initial channel query returns less messages than the custom query channels options message limit', async () => {
const { channel, chatClient } = await initClient();
useMockedApis(chatClient, [queryChannelWithNewMessages([generateMessage()], channel)]);
let hasMore;
const channelQueryOptions = {
messages: { limit: 10 },
};
await act(() => {
renderComponent(
{ channel, channelQueryOptions, chatClient },
({ hasMore: contextHasMore }) => {
hasMore = contextHasMore;
},
);
});

await waitFor(() => {
expect(hasMore).toBe(false);
});
});

it('should set hasMore state to true if the initial channel query returns count of messages equal custom query channels options message limit', async () => {
const { channel, chatClient } = await initClient();
const equalCount = 10;
useMockedApis(chatClient, [
queryChannelWithNewMessages(Array.from({ length: equalCount }, generateMessage), channel),
]);
let hasMore;
const channelQueryOptions = {
messages: { limit: equalCount },
};
await act(() => {
renderComponent(
{ channel, channelQueryOptions, chatClient },
({ hasMore: contextHasMore }) => {
hasMore = contextHasMore;
},
);
});

await waitFor(() => {
expect(hasMore).toBe(true);
});
});

it('should not call watch the current channel on mount if channel is initialized', async () => {
const { channel, chatClient } = await initClient();
const watchSpy = jest.spyOn(channel, 'watch');
Expand Down Expand Up @@ -605,19 +695,6 @@ describe('Channel', () => {
});

describe('loading more messages', () => {
const queryChannelWithNewMessages = (newMessages, channel) =>
// generate new channel mock from existing channel with new messages added
getOrCreateChannelApi(
generateChannel({
channel: {
config: channel.getConfig(),
id: channel.id,
type: channel.type,
},
messages: newMessages,
}),
);

const limit = 10;
it('should be able to load more messages', async () => {
const { channel, chatClient } = await initClient();
Expand Down Expand Up @@ -665,7 +742,7 @@ describe('Channel', () => {
useMockedApis(chatClient, [queryChannelWithNewMessages(newMessages, channel)]);
loadMore(limit);
} else {
// If message has been added, set our checker variable so we can verify if hasMore is false.
// If message has been added, set our checker variable, so we can verify if hasMore is false.
channelHasMore = hasMore;
}
},
Expand Down Expand Up @@ -713,6 +790,151 @@ describe('Channel', () => {
});
await waitFor(() => expect(isLoadingMore).toBe(true));
});

it('should not load the second page, if the previous query has returned less then default limit messages', async () => {
const { channel, chatClient } = await initClient();
const firstPageOfMessages = [generateMessage()];
useMockedApis(chatClient, [queryChannelWithNewMessages(firstPageOfMessages, channel)]);
let queryNextPageSpy;
let contextMessageCount;
await act(() => {
renderComponent({ channel, chatClient }, ({ loadMore, messages: contextMessages }) => {
queryNextPageSpy = jest.spyOn(channel, 'query');
contextMessageCount = contextMessages.length;
loadMore();
});
});

await waitFor(() => {
expect(queryNextPageSpy).not.toHaveBeenCalled();
expect(chatClient.axiosInstance.post).toHaveBeenCalledTimes(1);
expect(chatClient.axiosInstance.post.mock.calls[0][1]).toMatchObject(
expect.objectContaining({ data: {}, presence: false, state: true, watch: false }),
);
expect(contextMessageCount).toBe(firstPageOfMessages.length);
});
});
it('should load the second page, if the previous query has returned message count equal default messages limit', async () => {
const { channel, chatClient } = await initClient();
const firstPageMessages = Array.from({ length: 25 }, generateMessage);
const secondPageMessages = Array.from({ length: 15 }, generateMessage);
useMockedApis(chatClient, [queryChannelWithNewMessages(firstPageMessages, channel)]);
let queryNextPageSpy;
let contextMessageCount;
await act(() => {
renderComponent({ channel, chatClient }, ({ loadMore, messages: contextMessages }) => {
queryNextPageSpy = jest.spyOn(channel, 'query');
contextMessageCount = contextMessages.length;
useMockedApis(chatClient, [queryChannelWithNewMessages(secondPageMessages, channel)]);
loadMore();
});
});

await waitFor(() => {
expect(queryNextPageSpy).toHaveBeenCalledTimes(1);
expect(chatClient.axiosInstance.post).toHaveBeenCalledTimes(2);
expect(chatClient.axiosInstance.post.mock.calls[0][1]).toMatchObject({
data: {},
presence: false,
state: true,
watch: false,
});
expect(chatClient.axiosInstance.post.mock.calls[1][1]).toMatchObject(
expect.objectContaining({
data: {},
messages: { id_lt: firstPageMessages[0].id, limit: 100 },
state: true,
watchers: { limit: 100 },
}),
);
expect(contextMessageCount).toBe(firstPageMessages.length + secondPageMessages.length);
});
});
it('should not load the second page, if the previous query has returned less then custom limit messages', async () => {
const { channel, chatClient } = await initClient();
const channelQueryOptions = {
messages: { limit: 10 },
};
const firstPageOfMessages = [generateMessage()];
useMockedApis(chatClient, [queryChannelWithNewMessages(firstPageOfMessages, channel)]);
let queryNextPageSpy;
let contextMessageCount;
await act(() => {
renderComponent(
{ channel, channelQueryOptions, chatClient },
({ loadMore, messages: contextMessages }) => {
queryNextPageSpy = jest.spyOn(channel, 'query');
contextMessageCount = contextMessages.length;
loadMore(channelQueryOptions.messages.limit);
},
);
});

await waitFor(() => {
expect(queryNextPageSpy).not.toHaveBeenCalled();
expect(chatClient.axiosInstance.post).toHaveBeenCalledTimes(1);
expect(chatClient.axiosInstance.post.mock.calls[0][1]).toMatchObject({
data: {},
messages: {
limit: channelQueryOptions.messages.limit,
},
presence: false,
state: true,
watch: false,
});
expect(contextMessageCount).toBe(firstPageOfMessages.length);
});
});
it('should load the second page, if the previous query has returned message count equal custom messages limit', async () => {
const { channel, chatClient } = await initClient();
const equalCount = 10;
const channelQueryOptions = {
messages: { limit: equalCount },
};
const firstPageMessages = Array.from({ length: equalCount }, generateMessage);
const secondPageMessages = Array.from({ length: equalCount - 1 }, generateMessage);
useMockedApis(chatClient, [queryChannelWithNewMessages(firstPageMessages, channel)]);
let queryNextPageSpy;
let contextMessageCount;

await act(() => {
renderComponent(
{ channel, channelQueryOptions, chatClient },
({ loadMore, messages: contextMessages }) => {
queryNextPageSpy = jest.spyOn(channel, 'query');
contextMessageCount = contextMessages.length;
useMockedApis(chatClient, [queryChannelWithNewMessages(secondPageMessages, channel)]);
loadMore(channelQueryOptions.messages.limit);
},
);
});

await waitFor(() => {
expect(queryNextPageSpy).toHaveBeenCalledTimes(1);
expect(chatClient.axiosInstance.post).toHaveBeenCalledTimes(2);
expect(chatClient.axiosInstance.post.mock.calls[0][1]).toMatchObject({
data: {},
messages: {
limit: channelQueryOptions.messages.limit,
},
presence: false,
state: true,
watch: false,
});
expect(chatClient.axiosInstance.post.mock.calls[1][1]).toMatchObject(
expect.objectContaining({
data: {},
messages: {
id_lt: firstPageMessages[0].id,
limit: channelQueryOptions.messages.limit,
},
state: true,
watchers: { limit: channelQueryOptions.messages.limit },
}),
);
expect(contextMessageCount).toBe(firstPageMessages.length + secondPageMessages.length);
});
});
});

describe('Sending/removing/updating messages', () => {
Expand Down
4 changes: 3 additions & 1 deletion src/components/Channel/channelState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export type ChannelStateReducerAction<
}
| {
channel: Channel<StreamChatGenerics>;
hasMore: boolean;
type: 'initStateFromChannel';
}
| {
Expand Down Expand Up @@ -132,9 +133,10 @@ export const channelReducer = <
}

case 'initStateFromChannel': {
const { channel } = action;
const { channel, hasMore } = action;
return {
...state,
hasMore,
loading: false,
members: { ...channel.state.members },
messages: [...channel.state.messages],
Expand Down
2 changes: 1 addition & 1 deletion src/components/InfiniteScrollPaginator/InfiniteScroll.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export const InfiniteScroll = (props: PropsWithChildren<InfiniteScrollProps>) =>
}

if (isLoading) return;

// FIXME: this triggers loadMore call when a user types messages in thread and the scroll container container expands
if (
reverseOffset < Number(threshold) &&
typeof loadPreviousPageFn === 'function' &&
Expand Down
1 change: 1 addition & 0 deletions src/components/MessageList/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -318,5 +318,6 @@ export const getGroupStyles = <
export const hasMoreMessagesProbably = (returnedCountMessages: number, limit: number) =>
returnedCountMessages === limit;

// @deprecated
export const hasNotMoreMessages = (returnedCountMessages: number, limit: number) =>
returnedCountMessages < limit;
Loading
Loading