From 71f778ed99237d9cbaec8a7a3bdd5580518c6ca6 Mon Sep 17 00:00:00 2001 From: martincupela Date: Mon, 20 Nov 2023 15:56:16 +0100 Subject: [PATCH] fix: calculate pagination stop from custom channel query message limit --- src/components/Channel/Channel.tsx | 26 +- .../Channel/__tests__/Channel.test.js | 250 +++++++++++++++++- src/components/Channel/channelState.ts | 4 +- .../InfiniteScroll.tsx | 2 +- src/components/MessageList/utils.ts | 1 + src/components/Thread/Thread.tsx | 1 + 6 files changed, 255 insertions(+), 29 deletions(-) diff --git a/src/components/Channel/Channel.tsx b/src/components/Channel/Channel.tsx index 81f9d0aa0..1b8700a31 100644 --- a/src/components/Channel/Channel.tsx +++ b/src/components/Channel/Channel.tsx @@ -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'; @@ -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 }, @@ -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); @@ -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]; @@ -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; @@ -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({ @@ -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' }); diff --git a/src/components/Channel/__tests__/Channel.test.js b/src/components/Channel/__tests__/Channel.test.js index 7b201b63a..92a2d5678 100644 --- a/src/components/Channel/__tests__/Channel.test.js +++ b/src/components/Channel/__tests__/Channel.test.js @@ -33,6 +33,19 @@ jest.mock('../../Loading', () => ({ LoadingIndicator: jest.fn(() =>
loading
), })); +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 }) => (
{name} @@ -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'); @@ -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(); @@ -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; } }, @@ -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', () => { diff --git a/src/components/Channel/channelState.ts b/src/components/Channel/channelState.ts index 415d200eb..44cd430fd 100644 --- a/src/components/Channel/channelState.ts +++ b/src/components/Channel/channelState.ts @@ -30,6 +30,7 @@ export type ChannelStateReducerAction< } | { channel: Channel; + hasMore: boolean; type: 'initStateFromChannel'; } | { @@ -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], diff --git a/src/components/InfiniteScrollPaginator/InfiniteScroll.tsx b/src/components/InfiniteScrollPaginator/InfiniteScroll.tsx index 445474d61..aad3c278f 100644 --- a/src/components/InfiniteScrollPaginator/InfiniteScroll.tsx +++ b/src/components/InfiniteScrollPaginator/InfiniteScroll.tsx @@ -91,7 +91,7 @@ export const InfiniteScroll = (props: PropsWithChildren) => } 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' && diff --git a/src/components/MessageList/utils.ts b/src/components/MessageList/utils.ts index 90d361fcb..95cdc54b3 100644 --- a/src/components/MessageList/utils.ts +++ b/src/components/MessageList/utils.ts @@ -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; diff --git a/src/components/Thread/Thread.tsx b/src/components/Thread/Thread.tsx index ba9e7f67e..2fe58fdeb 100644 --- a/src/components/Thread/Thread.tsx +++ b/src/components/Thread/Thread.tsx @@ -125,6 +125,7 @@ const ThreadInner = < useEffect(() => { if (thread?.id && thread?.reply_count) { + // FIXME: integrators can customize channel query options but cannot customize channel.getReplies() options loadMoreThread(); } }, []);