From 5387d31a68de04d46a7da64379db9b8c93744ffc Mon Sep 17 00:00:00 2001 From: Diego Mello Date: Fri, 13 Jan 2023 16:32:52 -0300 Subject: [PATCH] [FIX] Messages not loading on some edge cases (#4801) --- app/actions/actionsTypes.ts | 4 +- app/actions/room.ts | 37 +++++++- app/containers/message/index.tsx | 3 +- app/definitions/index.ts | 1 + .../methods}/getMoreMessages.ts | 9 +- app/lib/methods/index.ts | 1 + app/lib/methods/loadNextMessages.ts | 2 - app/lib/methods/loadSurroundingMessages.ts | 5 -- app/reducers/room.test.ts | 23 ++++- app/reducers/room.ts | 14 ++- app/sagas/room.js | 23 ++++- app/sagas/rooms.js | 2 +- .../RoomView/LoadMore/LoadMore.stories.tsx | 20 ++--- app/views/RoomView/LoadMore/index.tsx | 90 +++++++++---------- app/views/RoomView/index.tsx | 51 +++++------ app/views/RoomView/services/getMessages.ts | 33 ++++--- app/views/RoomView/services/index.ts | 2 - 17 files changed, 198 insertions(+), 122 deletions(-) rename app/{views/RoomView/services => lib/methods}/getMoreMessages.ts (70%) diff --git a/app/actions/actionsTypes.ts b/app/actions/actionsTypes.ts index f33c9ec7f..01c707453 100644 --- a/app/actions/actionsTypes.ts +++ b/app/actions/actionsTypes.ts @@ -28,7 +28,9 @@ export const ROOM = createRequestTypes('ROOM', [ 'DELETE', 'REMOVED', 'FORWARD', - 'USER_TYPING' + 'USER_TYPING', + 'HISTORY_REQUEST', + 'HISTORY_FINISHED' ]); export const INQUIRY = createRequestTypes('INQUIRY', [ ...defaultTypes, diff --git a/app/actions/room.ts b/app/actions/room.ts index 168110199..901aeba47 100644 --- a/app/actions/room.ts +++ b/app/actions/room.ts @@ -1,6 +1,6 @@ import { Action } from 'redux'; -import { ERoomType } from '../definitions/ERoomType'; +import { ERoomType, RoomType } from '../definitions'; import { ROOM } from './actionsTypes'; // TYPE RETURN RELATED @@ -44,7 +44,24 @@ interface IUserTyping extends Action { status: boolean; } -export type TActionsRoom = TSubscribeRoom & TUnsubscribeRoom & ILeaveRoom & IDeleteRoom & IForwardRoom & IUserTyping; +export interface IRoomHistoryRequest extends Action { + rid: string; + t: RoomType; + loaderId: string; +} + +export interface IRoomHistoryFinished extends Action { + loaderId: string; +} + +export type TActionsRoom = TSubscribeRoom & + TUnsubscribeRoom & + ILeaveRoom & + IDeleteRoom & + IForwardRoom & + IUserTyping & + IRoomHistoryRequest & + IRoomHistoryFinished; export function subscribeRoom(rid: string): TSubscribeRoom { return { @@ -99,3 +116,19 @@ export function userTyping(rid: string, status = true): IUserTyping { status }; } + +export function roomHistoryRequest({ rid, t, loaderId }: { rid: string; t: RoomType; loaderId: string }): IRoomHistoryRequest { + return { + type: ROOM.HISTORY_REQUEST, + rid, + t, + loaderId + }; +} + +export function roomHistoryFinished({ loaderId }: { loaderId: string }): IRoomHistoryFinished { + return { + type: ROOM.HISTORY_FINISHED, + loaderId + }; +} diff --git a/app/containers/message/index.tsx b/app/containers/message/index.tsx index 246284536..8595d5d0a 100644 --- a/app/containers/message/index.tsx +++ b/app/containers/message/index.tsx @@ -407,7 +407,8 @@ class MessageContainer extends React.Component + }} + > {/* @ts-ignore*/} , S extends string> { navigation: StackNavigationProp; diff --git a/app/views/RoomView/services/getMoreMessages.ts b/app/lib/methods/getMoreMessages.ts similarity index 70% rename from app/views/RoomView/services/getMoreMessages.ts rename to app/lib/methods/getMoreMessages.ts index 6e4d007a4..c82fd74a9 100644 --- a/app/views/RoomView/services/getMoreMessages.ts +++ b/app/lib/methods/getMoreMessages.ts @@ -1,16 +1,14 @@ -import { SubscriptionType, TAnyMessageModel } from '../../../definitions'; -import { loadNextMessages, loadMessagesForRoom } from '../../../lib/methods'; -import { MessageTypeLoad } from '../../../lib/constants'; +import { SubscriptionType, TAnyMessageModel } from '../../definitions'; +import { loadNextMessages, loadMessagesForRoom } from '.'; +import { MessageTypeLoad } from '../constants'; const getMoreMessages = ({ rid, t, - tmid, loaderItem }: { rid: string; t: SubscriptionType; - tmid?: string; loaderItem: TAnyMessageModel; }): Promise => { if ([MessageTypeLoad.MORE, MessageTypeLoad.PREVIOUS_CHUNK].includes(loaderItem.t as MessageTypeLoad)) { @@ -25,7 +23,6 @@ const getMoreMessages = ({ if (loaderItem.t === MessageTypeLoad.NEXT_CHUNK) { return loadNextMessages({ rid, - tmid, ts: loaderItem.ts as Date, loaderItem }); diff --git a/app/lib/methods/index.ts b/app/lib/methods/index.ts index e8b0229b1..602d78f16 100644 --- a/app/lib/methods/index.ts +++ b/app/lib/methods/index.ts @@ -16,6 +16,7 @@ export * from './getSingleMessage'; export * from './getSlashCommands'; export * from './getThreadName'; export * from './getUsersPresence'; +export * from './getMoreMessages'; export * from './loadMessagesForRoom'; export * from './loadMissedMessages'; export * from './loadNextMessages'; diff --git a/app/lib/methods/loadNextMessages.ts b/app/lib/methods/loadNextMessages.ts index 5a650c732..c221e9577 100644 --- a/app/lib/methods/loadNextMessages.ts +++ b/app/lib/methods/loadNextMessages.ts @@ -15,7 +15,6 @@ const COUNT = 50; interface ILoadNextMessages { rid: string; ts: Date; - tmid?: string; loaderItem: TMessageModel; } @@ -32,7 +31,6 @@ export function loadNextMessages(args: ILoadNextMessages): Promise { const loadMoreItem = { _id: generateLoadMoreId(lastMessage._id), rid: lastMessage.rid, - tmid: args.tmid, ts: moment(lastMessage.ts).add(1, 'millisecond'), t: MessageTypeLoad.NEXT_CHUNK }; diff --git a/app/lib/methods/loadSurroundingMessages.ts b/app/lib/methods/loadSurroundingMessages.ts index a1017b219..a3c31be89 100644 --- a/app/lib/methods/loadSurroundingMessages.ts +++ b/app/lib/methods/loadSurroundingMessages.ts @@ -19,9 +19,6 @@ export function loadSurroundingMessages({ messageId, rid }: { messageId: string; let messages: IMessage[] = EJSON.fromJSONValue(data?.messages); messages = orderBy(messages, 'ts'); - const message = messages.find(m => m._id === messageId); - const tmid = message?.tmid; - if (messages?.length) { if (data?.moreBefore) { const firstMessage = messages[0]; @@ -30,7 +27,6 @@ export function loadSurroundingMessages({ messageId, rid }: { messageId: string; const loadMoreItem = { _id: generateLoadMoreId(firstMessage._id), rid: firstMessage.rid, - tmid, ts: moment(firstMessage.ts).subtract(1, 'millisecond').toDate(), t: MessageTypeLoad.PREVIOUS_CHUNK, msg: firstMessage.msg @@ -46,7 +42,6 @@ export function loadSurroundingMessages({ messageId, rid }: { messageId: string; const loadMoreItem = { _id: generateLoadMoreId(lastMessage._id), rid: lastMessage.rid, - tmid, ts: moment(lastMessage.ts).add(1, 'millisecond').toDate(), t: MessageTypeLoad.NEXT_CHUNK, msg: lastMessage.msg diff --git a/app/reducers/room.test.ts b/app/reducers/room.test.ts index 683bf2881..8d9620e25 100644 --- a/app/reducers/room.test.ts +++ b/app/reducers/room.test.ts @@ -1,4 +1,13 @@ -import { deleteRoom, forwardRoom, leaveRoom, removedRoom, subscribeRoom, unsubscribeRoom } from '../actions/room'; +import { + deleteRoom, + forwardRoom, + leaveRoom, + removedRoom, + roomHistoryFinished, + roomHistoryRequest, + subscribeRoom, + unsubscribeRoom +} from '../actions/room'; import { ERoomType } from '../definitions/ERoomType'; import { mockedStore } from './mockedStore'; import { initialState } from './room'; @@ -48,4 +57,16 @@ describe('test room reducer', () => { const { isDeleting } = mockedStore.getState().room; expect(isDeleting).toEqual(false); }); + + it('should return historyLoaders with one item after call historyRequest', () => { + mockedStore.dispatch(roomHistoryRequest({ rid: 'GENERAL', t: 'c', loaderId: 'loader' })); + const { historyLoaders } = mockedStore.getState().room; + expect(historyLoaders).toEqual(['loader']); + }); + + it('should return historyLoaders with empty array after call historyFinished', () => { + mockedStore.dispatch(roomHistoryFinished({ loaderId: 'loader' })); + const { historyLoaders } = mockedStore.getState().room; + expect(historyLoaders).toEqual([]); + }); }); diff --git a/app/reducers/room.ts b/app/reducers/room.ts index 3462247d3..c988d8683 100644 --- a/app/reducers/room.ts +++ b/app/reducers/room.ts @@ -7,12 +7,14 @@ export interface IRoom { rid: string; isDeleting: boolean; subscribedRoom: string; + historyLoaders: string[]; } export const initialState: IRoom = { rid: '', isDeleting: false, - subscribedRoom: '' + subscribedRoom: '', + historyLoaders: [] }; export default function (state = initialState, action: TActionsRoom): IRoom { @@ -56,6 +58,16 @@ export default function (state = initialState, action: TActionsRoom): IRoom { ...state, isDeleting: false }; + case ROOM.HISTORY_REQUEST: + return { + ...state, + historyLoaders: [...state.historyLoaders, action.loaderId] + }; + case ROOM.HISTORY_FINISHED: + return { + ...state, + historyLoaders: state.historyLoaders.filter(loaderId => loaderId !== action.loaderId) + }; default: return state; } diff --git a/app/sagas/room.js b/app/sagas/room.js index 6f358ead8..89c0e7677 100644 --- a/app/sagas/room.js +++ b/app/sagas/room.js @@ -1,5 +1,5 @@ import { Alert } from 'react-native'; -import { delay, put, race, select, take, takeLatest } from 'redux-saga/effects'; +import { delay, put, race, select, take, takeLatest, actionChannel } from 'redux-saga/effects'; import EventEmitter from '../lib/methods/helpers/events'; import Navigation from '../lib/navigation/appNavigation'; @@ -10,6 +10,26 @@ import I18n from '../i18n'; import { showErrorAlert } from '../lib/methods/helpers/info'; import { LISTENER } from '../containers/Toast'; import { Services } from '../lib/services'; +import getMoreMessages from '../lib/methods/getMoreMessages'; +import { getMessageById } from '../lib/database/services/Message'; + +function* watchHistoryRequests() { + const requestChan = yield actionChannel(types.ROOM.HISTORY_REQUEST); + while (true) { + const { rid, t, tmid, loaderId } = yield take(requestChan); + + const loaderItem = yield getMessageById(loaderId); + if (loaderItem) { + try { + yield getMoreMessages({ rid, t, tmid, loaderItem }); + } catch (e) { + log(e); + } finally { + yield put({ type: types.ROOM.HISTORY_FINISHED, loaderId }); + } + } + } +} const watchUserTyping = function* watchUserTyping({ rid, status }) { const auth = yield select(state => state.login.isAuthenticated); @@ -132,5 +152,6 @@ const root = function* root() { yield takeLatest(types.ROOM.LEAVE, handleLeaveRoom); yield takeLatest(types.ROOM.DELETE, handleDeleteRoom); yield takeLatest(types.ROOM.FORWARD, handleForwardRoom); + yield watchHistoryRequests(); }; export default root; diff --git a/app/sagas/rooms.js b/app/sagas/rooms.js index 6f2bc73da..66e601e48 100644 --- a/app/sagas/rooms.js +++ b/app/sagas/rooms.js @@ -66,7 +66,7 @@ const handleRoomsRequest = function* handleRoomsRequest({ params }) { */ .filter(sub => subscribedRoom !== sub.rid) .map(sub => sub.lastMessage && buildMessage(sub.lastMessage)) - .filter(lm => lm); + .filter(lm => lm && lm._id && lm.rid); const lastMessagesIds = lastMessages.map(lm => lm._id).filter(lm => lm); const existingMessages = yield messagesCollection.query(Q.where('id', Q.oneOf(lastMessagesIds))).fetch(); const messagesToUpdate = existingMessages.filter(i1 => lastMessages.find(i2 => i1.id === i2._id)); diff --git a/app/views/RoomView/LoadMore/LoadMore.stories.tsx b/app/views/RoomView/LoadMore/LoadMore.stories.tsx index affbad3ad..c95475823 100644 --- a/app/views/RoomView/LoadMore/LoadMore.stories.tsx +++ b/app/views/RoomView/LoadMore/LoadMore.stories.tsx @@ -11,28 +11,28 @@ export default { title: 'RoomView/LoadMore' }; -const load = () => new Promise(res => setTimeout(res, 1000)); - -const LoadMore = ({ ...props }) => ; +const LoadMore = ({ ...props }) => ( + +); export const Basic = () => ( <> - - - - + + + + ); const ThemeStory = ({ theme }: { theme: TSupportedThemes }) => ( - + - - + + diff --git a/app/views/RoomView/LoadMore/index.tsx b/app/views/RoomView/LoadMore/index.tsx index d08d82460..78689923e 100644 --- a/app/views/RoomView/LoadMore/index.tsx +++ b/app/views/RoomView/LoadMore/index.tsx @@ -1,12 +1,15 @@ -import React, { useCallback, useEffect, useState } from 'react'; +import React, { useEffect } from 'react'; import { ActivityIndicator, StyleSheet, Text } from 'react-native'; +import { useDispatch } from 'react-redux'; -import { MessageTypeLoad, themes } from '../../../lib/constants'; -import { MessageType } from '../../../definitions'; +import { MessageTypeLoad } from '../../../lib/constants'; +import { MessageType, RoomType } from '../../../definitions'; import { useTheme } from '../../../theme'; import Touch from '../../../containers/Touch'; import sharedStyles from '../../Styles'; import I18n from '../../../i18n'; +import { roomHistoryRequest } from '../../../actions/room'; +import { useAppSelector } from '../../../lib/hooks'; const styles = StyleSheet.create({ button: { @@ -20,53 +23,50 @@ const styles = StyleSheet.create({ } }); -const LoadMore = ({ - load, - type, - runOnRender -}: { - load: Function; - type: MessageType; - runOnRender: boolean; -}): React.ReactElement => { - const { theme } = useTheme(); - const [loading, setLoading] = useState(false); +const LoadMore = React.memo( + ({ + rid, + t, + loaderId, + type, + runOnRender + }: { + rid: string; + t: RoomType; + loaderId: string; + type: MessageType; + runOnRender: boolean; + }): React.ReactElement => { + const { colors } = useTheme(); + const dispatch = useDispatch(); + const loading = useAppSelector(state => state.room.historyLoaders.some(historyLoader => historyLoader === loaderId)); - const handleLoad = useCallback(async () => { - try { - if (loading) { - return; + const handleLoad = () => dispatch(roomHistoryRequest({ rid, t, loaderId })); + + useEffect(() => { + if (runOnRender) { + handleLoad(); } - setLoading(true); - await load(); - } finally { - setLoading(false); + }, []); + + let text = 'Load_More'; + if (type === MessageTypeLoad.NEXT_CHUNK) { + text = 'Load_Newer'; } - }, [loading]); - - useEffect(() => { - if (runOnRender) { - handleLoad(); + if (type === MessageTypeLoad.PREVIOUS_CHUNK) { + text = 'Load_Older'; } - }, []); - let text = 'Load_More'; - if (type === MessageTypeLoad.NEXT_CHUNK) { - text = 'Load_Newer'; + return ( + + {loading ? ( + + ) : ( + {I18n.t(text)} + )} + + ); } - if (type === MessageTypeLoad.PREVIOUS_CHUNK) { - text = 'Load_Older'; - } - - return ( - - {loading ? ( - - ) : ( - {I18n.t(text)} - )} - - ); -}; +); export default LoadMore; diff --git a/app/views/RoomView/index.tsx b/app/views/RoomView/index.tsx index ca8e7980d..92f247f23 100644 --- a/app/views/RoomView/index.tsx +++ b/app/views/RoomView/index.tsx @@ -75,7 +75,8 @@ import { TThreadModel, ICustomEmojis, IEmoji, - TGetCustomEmoji + TGetCustomEmoji, + RoomType } from '../../definitions'; import { E2E_MESSAGE_TYPE, E2E_STATUS, MESSAGE_TYPE_ANY_LOAD, MessageTypeLoad, themes } from '../../lib/constants'; import { TListRef } from './List/List'; @@ -174,18 +175,18 @@ interface IRoomViewState { [key: string]: any; joined: boolean; room: - | TSubscriptionModel - | { - rid: string; - t: string; - name?: string; - fname?: string; - prid?: string; - joinCodeRequired?: boolean; - status?: string; - lastMessage?: ILastMessage; - sysMes?: boolean; - onHold?: boolean; + | TSubscriptionModel + | { + rid: string; + t: string; + name?: string; + fname?: string; + prid?: string; + joinCodeRequired?: boolean; + status?: string; + lastMessage?: ILastMessage; + sysMes?: boolean; + onHold?: boolean; }; roomUpdate: { [K in TRoomUpdate]?: any; @@ -685,7 +686,11 @@ class RoomView extends React.Component { await loadThreadMessages({ tmid: this.tmid, rid: this.rid }); } else { const newLastOpen = new Date(); - await RoomServices.getMessages(room); + await RoomServices.getMessages({ + rid: room.rid, + lastOpen: 'lastOpen' in room ? room.lastOpen : undefined, + t: room.t as RoomType + }); // if room is joined if (joined && 'id' in room) { @@ -1301,16 +1306,6 @@ class RoomView extends React.Component { return false; }; - onLoadMoreMessages = (loaderItem: TAnyMessageModel) => { - const { room } = this.state; - return RoomServices.getMoreMessages({ - rid: room.rid, - tmid: this.tmid, - t: room.t as any, - loaderItem - }); - }; - goToCannedResponses = () => { const { room } = this.state; Navigation.navigate('CannedResponsesListView', { rid: room.rid }); @@ -1337,7 +1332,9 @@ class RoomView extends React.Component { if (item.t && MESSAGE_TYPE_ANY_LOAD.includes(item.t as MessageTypeLoad)) { content = ( this.onLoadMoreMessages(item)} + rid={room.rid} + t={room.t as RoomType} + loaderId={item.id} type={item.t} runOnRender={item.t === MessageTypeLoad.MORE && !previousItem} /> @@ -1502,7 +1499,7 @@ class RoomView extends React.Component { return ( <> this.messageActions = ref} + ref={ref => (this.messageActions = ref)} tmid={this.tmid} room={room} user={user} @@ -1512,7 +1509,7 @@ class RoomView extends React.Component { onReactionPress={this.onReactionPress} isReadOnly={readOnly} /> - this.messageErrorActions = ref} tmid={this.tmid} /> + (this.messageErrorActions = ref)} tmid={this.tmid} /> ); }; diff --git a/app/views/RoomView/services/getMessages.ts b/app/views/RoomView/services/getMessages.ts index 8bb298625..b05fadeff 100644 --- a/app/views/RoomView/services/getMessages.ts +++ b/app/views/RoomView/services/getMessages.ts @@ -1,23 +1,22 @@ -import { loadMessagesForRoom, loadMissedMessages } from '../../../lib/methods'; +import { loadMessagesForRoom, loadMissedMessages, RoomTypes } from '../../../lib/methods'; -// TODO: clarify latest vs lastOpen -const getMessages = ({ - rid, - t, - latest, - lastOpen, - loaderItem -}: { +interface IBaseParams { rid: string; - t?: string; - latest?: Date; - lastOpen?: Date; - loaderItem?: any; // TODO: type this -}): Promise => { - if (lastOpen) { - return loadMissedMessages({ rid, lastOpen }); +} + +interface ILoadMessagesForRoomParams extends IBaseParams { + t: RoomTypes; +} + +interface ILoadMissedMessagesParams extends IBaseParams { + lastOpen: Date; +} + +const getMessages = (params: ILoadMissedMessagesParams | ILoadMessagesForRoomParams): Promise => { + if ('lastOpen' in params) { + return loadMissedMessages(params); } - return loadMessagesForRoom({ rid, t: t as any, latest, loaderItem }); + return loadMessagesForRoom(params); }; export default getMessages; diff --git a/app/views/RoomView/services/index.ts b/app/views/RoomView/services/index.ts index 47565eabe..aeb43669f 100644 --- a/app/views/RoomView/services/index.ts +++ b/app/views/RoomView/services/index.ts @@ -1,9 +1,7 @@ import getMessages from './getMessages'; -import getMoreMessages from './getMoreMessages'; import getMessageInfo from './getMessageInfo'; export default { getMessages, - getMoreMessages, getMessageInfo };