Skip to content

Commit 8374af1

Browse files
MartinCupelaMartinCupela
authored
fix: calculate pagination stop from custom channel query message limit (#2180)
1 parent 1d3c1ff commit 8374af1

File tree

6 files changed

+255
-29
lines changed

6 files changed

+255
-29
lines changed

‎src/components/Channel/Channel.tsx‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ import {
6868
DEFAULT_THREAD_PAGE_SIZE,
6969
} from '../../constants/limits';
7070

71-
import { hasMoreMessagesProbably, hasNotMoreMessages } from '../MessageList/utils';
71+
import { hasMoreMessagesProbably } from '../MessageList/utils';
7272
import defaultEmojiData from '../../stream-emoji.json';
7373
import { makeAddNotifications } from './utils';
7474
import { getChannel } from '../../utils/getChannel';
@@ -490,6 +490,7 @@ const ChannelInner = <
490490
/**
491491
* 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.
492492
*/
493+
// FIXME: we should use channelQueryOptions if they are available
493494
await channel.query({
494495
messages: { id_lt: oldestID, limit: DEFAULT_NEXT_CHANNEL_PAGE_SIZE },
495496
watchers: { limit: DEFAULT_NEXT_CHANNEL_PAGE_SIZE },
@@ -542,7 +543,14 @@ const ChannelInner = <
542543
originalTitle.current = document.title;
543544

544545
if (!errored) {
545-
dispatch({ channel, type: 'initStateFromChannel' });
546+
dispatch({
547+
channel,
548+
hasMore: hasMoreMessagesProbably(
549+
channel.state.messages.length,
550+
channelQueryOptions?.messages?.limit ?? DEFAULT_INITIAL_CHANNEL_PAGE_SIZE,
551+
),
552+
type: 'initStateFromChannel',
553+
});
546554
if (channel.countUnread() > 0) markRead();
547555
// The more complex sync logic is done in Chat
548556
document.addEventListener('visibilitychange', onVisibilityChange);
@@ -598,7 +606,7 @@ const ChannelInner = <
598606
);
599607

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

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

610-
// initial state loads with up to 25 messages, so if less than 25 no need for additional query
611-
const notHasMore = hasNotMoreMessages(
612-
channel.state.messages.length,
613-
DEFAULT_INITIAL_CHANNEL_PAGE_SIZE,
614-
);
615-
if (notHasMore) {
616-
loadMoreFinished(false, channel.state.messages);
617-
return channel.state.messages.length;
618-
}
619-
620618
dispatch({ loadingMore: true, type: 'setLoadingMore' });
621619

622620
const oldestID = oldestMessage?.id;
@@ -701,6 +699,7 @@ const ChannelInner = <
701699

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

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

918918
dispatch({ type: 'startLoadingThread' });

‎src/components/Channel/__tests__/Channel.test.js‎

Lines changed: 236 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,19 @@ jest.mock('../../Loading', () => ({
3333
LoadingIndicator: jest.fn(() => <div>loading</div>),
3434
}));
3535

36+
const queryChannelWithNewMessages = (newMessages, channel) =>
37+
// generate new channel mock from existing channel with new messages added
38+
getOrCreateChannelApi(
39+
generateChannel({
40+
channel: {
41+
config: channel.getConfig(),
42+
id: channel.id,
43+
type: channel.type,
44+
},
45+
messages: newMessages,
46+
}),
47+
);
48+
3649
const MockAvatar = ({ name }) => (
3750
<div className='avatar' data-testid='custom-avatar'>
3851
{name}
@@ -294,6 +307,83 @@ describe('Channel', () => {
294307
});
295308
});
296309

310+
it('should set hasMore state to false if the initial channel query returns less messages than the default initial page size', async () => {
311+
const { channel, chatClient } = await initClient();
312+
useMockedApis(chatClient, [queryChannelWithNewMessages([generateMessage()], channel)]);
313+
let hasMore;
314+
await act(() => {
315+
renderComponent({ channel, chatClient }, ({ hasMore: contextHasMore }) => {
316+
hasMore = contextHasMore;
317+
});
318+
});
319+
320+
await waitFor(() => {
321+
expect(hasMore).toBe(false);
322+
});
323+
});
324+
325+
it('should set hasMore state to true if the initial channel query returns count of messages equal to the default initial page size', async () => {
326+
const { channel, chatClient } = await initClient();
327+
useMockedApis(chatClient, [
328+
queryChannelWithNewMessages(Array.from({ length: 25 }, generateMessage), channel),
329+
]);
330+
let hasMore;
331+
await act(() => {
332+
renderComponent({ channel, chatClient }, ({ hasMore: contextHasMore }) => {
333+
hasMore = contextHasMore;
334+
});
335+
});
336+
337+
await waitFor(() => {
338+
expect(hasMore).toBe(true);
339+
});
340+
});
341+
342+
it('should set hasMore state to false if the initial channel query returns less messages than the custom query channels options message limit', async () => {
343+
const { channel, chatClient } = await initClient();
344+
useMockedApis(chatClient, [queryChannelWithNewMessages([generateMessage()], channel)]);
345+
let hasMore;
346+
const channelQueryOptions = {
347+
messages: { limit: 10 },
348+
};
349+
await act(() => {
350+
renderComponent(
351+
{ channel, channelQueryOptions, chatClient },
352+
({ hasMore: contextHasMore }) => {
353+
hasMore = contextHasMore;
354+
},
355+
);
356+
});
357+
358+
await waitFor(() => {
359+
expect(hasMore).toBe(false);
360+
});
361+
});
362+
363+
it('should set hasMore state to true if the initial channel query returns count of messages equal custom query channels options message limit', async () => {
364+
const { channel, chatClient } = await initClient();
365+
const equalCount = 10;
366+
useMockedApis(chatClient, [
367+
queryChannelWithNewMessages(Array.from({ length: equalCount }, generateMessage), channel),
368+
]);
369+
let hasMore;
370+
const channelQueryOptions = {
371+
messages: { limit: equalCount },
372+
};
373+
await act(() => {
374+
renderComponent(
375+
{ channel, channelQueryOptions, chatClient },
376+
({ hasMore: contextHasMore }) => {
377+
hasMore = contextHasMore;
378+
},
379+
);
380+
});
381+
382+
await waitFor(() => {
383+
expect(hasMore).toBe(true);
384+
});
385+
});
386+
297387
it('should not call watch the current channel on mount if channel is initialized', async () => {
298388
const { channel, chatClient } = await initClient();
299389
const watchSpy = jest.spyOn(channel, 'watch');
@@ -605,19 +695,6 @@ describe('Channel', () => {
605695
});
606696

607697
describe('loading more messages', () => {
608-
const queryChannelWithNewMessages = (newMessages, channel) =>
609-
// generate new channel mock from existing channel with new messages added
610-
getOrCreateChannelApi(
611-
generateChannel({
612-
channel: {
613-
config: channel.getConfig(),
614-
id: channel.id,
615-
type: channel.type,
616-
},
617-
messages: newMessages,
618-
}),
619-
);
620-
621698
const limit = 10;
622699
it('should be able to load more messages', async () => {
623700
const { channel, chatClient } = await initClient();
@@ -665,7 +742,7 @@ describe('Channel', () => {
665742
useMockedApis(chatClient, [queryChannelWithNewMessages(newMessages, channel)]);
666743
loadMore(limit);
667744
} else {
668-
// If message has been added, set our checker variable so we can verify if hasMore is false.
745+
// If message has been added, set our checker variable, so we can verify if hasMore is false.
669746
channelHasMore = hasMore;
670747
}
671748
},
@@ -713,6 +790,151 @@ describe('Channel', () => {
713790
});
714791
await waitFor(() => expect(isLoadingMore).toBe(true));
715792
});
793+
794+
it('should not load the second page, if the previous query has returned less then default limit messages', async () => {
795+
const { channel, chatClient } = await initClient();
796+
const firstPageOfMessages = [generateMessage()];
797+
useMockedApis(chatClient, [queryChannelWithNewMessages(firstPageOfMessages, channel)]);
798+
let queryNextPageSpy;
799+
let contextMessageCount;
800+
await act(() => {
801+
renderComponent({ channel, chatClient }, ({ loadMore, messages: contextMessages }) => {
802+
queryNextPageSpy = jest.spyOn(channel, 'query');
803+
contextMessageCount = contextMessages.length;
804+
loadMore();
805+
});
806+
});
807+
808+
await waitFor(() => {
809+
expect(queryNextPageSpy).not.toHaveBeenCalled();
810+
expect(chatClient.axiosInstance.post).toHaveBeenCalledTimes(1);
811+
expect(chatClient.axiosInstance.post.mock.calls[0][1]).toMatchObject(
812+
expect.objectContaining({ data: {}, presence: false, state: true, watch: false }),
813+
);
814+
expect(contextMessageCount).toBe(firstPageOfMessages.length);
815+
});
816+
});
817+
it('should load the second page, if the previous query has returned message count equal default messages limit', async () => {
818+
const { channel, chatClient } = await initClient();
819+
const firstPageMessages = Array.from({ length: 25 }, generateMessage);
820+
const secondPageMessages = Array.from({ length: 15 }, generateMessage);
821+
useMockedApis(chatClient, [queryChannelWithNewMessages(firstPageMessages, channel)]);
822+
let queryNextPageSpy;
823+
let contextMessageCount;
824+
await act(() => {
825+
renderComponent({ channel, chatClient }, ({ loadMore, messages: contextMessages }) => {
826+
queryNextPageSpy = jest.spyOn(channel, 'query');
827+
contextMessageCount = contextMessages.length;
828+
useMockedApis(chatClient, [queryChannelWithNewMessages(secondPageMessages, channel)]);
829+
loadMore();
830+
});
831+
});
832+
833+
await waitFor(() => {
834+
expect(queryNextPageSpy).toHaveBeenCalledTimes(1);
835+
expect(chatClient.axiosInstance.post).toHaveBeenCalledTimes(2);
836+
expect(chatClient.axiosInstance.post.mock.calls[0][1]).toMatchObject({
837+
data: {},
838+
presence: false,
839+
state: true,
840+
watch: false,
841+
});
842+
expect(chatClient.axiosInstance.post.mock.calls[1][1]).toMatchObject(
843+
expect.objectContaining({
844+
data: {},
845+
messages: { id_lt: firstPageMessages[0].id, limit: 100 },
846+
state: true,
847+
watchers: { limit: 100 },
848+
}),
849+
);
850+
expect(contextMessageCount).toBe(firstPageMessages.length + secondPageMessages.length);
851+
});
852+
});
853+
it('should not load the second page, if the previous query has returned less then custom limit messages', async () => {
854+
const { channel, chatClient } = await initClient();
855+
const channelQueryOptions = {
856+
messages: { limit: 10 },
857+
};
858+
const firstPageOfMessages = [generateMessage()];
859+
useMockedApis(chatClient, [queryChannelWithNewMessages(firstPageOfMessages, channel)]);
860+
let queryNextPageSpy;
861+
let contextMessageCount;
862+
await act(() => {
863+
renderComponent(
864+
{ channel, channelQueryOptions, chatClient },
865+
({ loadMore, messages: contextMessages }) => {
866+
queryNextPageSpy = jest.spyOn(channel, 'query');
867+
contextMessageCount = contextMessages.length;
868+
loadMore(channelQueryOptions.messages.limit);
869+
},
870+
);
871+
});
872+
873+
await waitFor(() => {
874+
expect(queryNextPageSpy).not.toHaveBeenCalled();
875+
expect(chatClient.axiosInstance.post).toHaveBeenCalledTimes(1);
876+
expect(chatClient.axiosInstance.post.mock.calls[0][1]).toMatchObject({
877+
data: {},
878+
messages: {
879+
limit: channelQueryOptions.messages.limit,
880+
},
881+
presence: false,
882+
state: true,
883+
watch: false,
884+
});
885+
expect(contextMessageCount).toBe(firstPageOfMessages.length);
886+
});
887+
});
888+
it('should load the second page, if the previous query has returned message count equal custom messages limit', async () => {
889+
const { channel, chatClient } = await initClient();
890+
const equalCount = 10;
891+
const channelQueryOptions = {
892+
messages: { limit: equalCount },
893+
};
894+
const firstPageMessages = Array.from({ length: equalCount }, generateMessage);
895+
const secondPageMessages = Array.from({ length: equalCount - 1 }, generateMessage);
896+
useMockedApis(chatClient, [queryChannelWithNewMessages(firstPageMessages, channel)]);
897+
let queryNextPageSpy;
898+
let contextMessageCount;
899+
900+
await act(() => {
901+
renderComponent(
902+
{ channel, channelQueryOptions, chatClient },
903+
({ loadMore, messages: contextMessages }) => {
904+
queryNextPageSpy = jest.spyOn(channel, 'query');
905+
contextMessageCount = contextMessages.length;
906+
useMockedApis(chatClient, [queryChannelWithNewMessages(secondPageMessages, channel)]);
907+
loadMore(channelQueryOptions.messages.limit);
908+
},
909+
);
910+
});
911+
912+
await waitFor(() => {
913+
expect(queryNextPageSpy).toHaveBeenCalledTimes(1);
914+
expect(chatClient.axiosInstance.post).toHaveBeenCalledTimes(2);
915+
expect(chatClient.axiosInstance.post.mock.calls[0][1]).toMatchObject({
916+
data: {},
917+
messages: {
918+
limit: channelQueryOptions.messages.limit,
919+
},
920+
presence: false,
921+
state: true,
922+
watch: false,
923+
});
924+
expect(chatClient.axiosInstance.post.mock.calls[1][1]).toMatchObject(
925+
expect.objectContaining({
926+
data: {},
927+
messages: {
928+
id_lt: firstPageMessages[0].id,
929+
limit: channelQueryOptions.messages.limit,
930+
},
931+
state: true,
932+
watchers: { limit: channelQueryOptions.messages.limit },
933+
}),
934+
);
935+
expect(contextMessageCount).toBe(firstPageMessages.length + secondPageMessages.length);
936+
});
937+
});
716938
});
717939

718940
describe('Sending/removing/updating messages', () => {

‎src/components/Channel/channelState.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ export type ChannelStateReducerAction<
3030
}
3131
| {
3232
channel: Channel<StreamChatGenerics>;
33+
hasMore: boolean;
3334
type: 'initStateFromChannel';
3435
}
3536
| {
@@ -132,9 +133,10 @@ export const channelReducer = <
132133
}
133134

134135
case 'initStateFromChannel': {
135-
const { channel } = action;
136+
const { channel, hasMore } = action;
136137
return {
137138
...state,
139+
hasMore,
138140
loading: false,
139141
members: { ...channel.state.members },
140142
messages: [...channel.state.messages],

‎src/components/InfiniteScrollPaginator/InfiniteScroll.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ export const InfiniteScroll = (props: PropsWithChildren<InfiniteScrollProps>) =>
9191
}
9292

9393
if (isLoading) return;
94-
94+
// FIXME: this triggers loadMore call when a user types messages in thread and the scroll container container expands
9595
if (
9696
reverseOffset < Number(threshold) &&
9797
typeof loadPreviousPageFn === 'function' &&

‎src/components/MessageList/utils.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,5 +318,6 @@ export const getGroupStyles = <
318318
export const hasMoreMessagesProbably = (returnedCountMessages: number, limit: number) =>
319319
returnedCountMessages === limit;
320320

321+
// @deprecated
321322
export const hasNotMoreMessages = (returnedCountMessages: number, limit: number) =>
322323
returnedCountMessages < limit;

0 commit comments

Comments
 (0)