[native] Fetch latest messages in chat list
AbandonedPublic
Actions

Authored by michal on Jun 2 2023, 6:17 AM.

Details

Reviewers

inka
kamil
• kuba

Summary

ENG-3973
Stop showing threads without any messages. Instead when we have no more threads we can display, use the previously introduced endpoint to fetch the latest message for more threads.

Depends on D8073

Test Plan

Check on multiple different test accounts if threads are correctly fetched and they don't jump around. Check if searching for (not)existing users works and the chat is correctly displayed.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

michal created this revision.Jun 2 2023, 6:17 AM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptJun 2 2023, 6:17 AM

Harbormaster returned this revision to the author for changes because remote builds failed.Jun 2 2023, 6:31 AM

Harbormaster failed remote builds in B19986: Diff 27397!

Fix CI

Harbormaster completed remote builds in B19987: Diff 27401.Jun 2 2023, 8:59 AM

michal requested review of this revision.Jun 2 2023, 8:59 AM

I patched this stack and I'm seeing some weird behaviour. Sometimes I cannot scroll further until I scroll back up and down again. Please investigate this

lib/hooks/message-hooks.js
75–88 ↗	(On Diff #27401)	`oldestMessage` is the message with the lowest id from results returned from `callFetchLatestMessages`. You set `setFetchState({ type: 'everything_loaded' });` when `oldestMessage === fetchState.message`, so when you see some message for the second time. Why do you see a message for the second time? Since `fetchThreadsWithLatestMessages` fetches messages that are strictly smaller than `fromMessageID` can this happen?

This revision now requires changes to proceed.Jun 5 2023, 5:26 AM

@inka heads-up, your upload doesn't appear to have attached

@ashoat thank you!

Updated to reflect changes in D8073, fixed an issue where sometimes threads wouldn't display.

@inka I couldn't reproduce your specific issue, but I suspect that changes to the endpoint in the previous diff will fix them. Could you try again if you have some time?

michal added inline comments.Jun 16 2023, 2:51 AM

native/chat/chat-thread-list.react.js
222–225	If we can't display any threads at the start, `onEndReached` is not called so we have to fetch manually (the `< 3` check takes into account the `search` and `empty` list items)

Harbormaster completed remote builds in B20303: Diff 27826.Jun 16 2023, 3:08 AM

michal added a child revision: D8234: [web] Use react-window.Jun 16 2023, 3:25 AM

Didn't do a close review, just nits

lib/hooks/message-hooks.js
28–29	React state should always be read-only
29	Nit
43–59	We can reduce indentation here and "exit early"

michal added inline comments.Jun 16 2023, 7:49 AM

lib/hooks/message-hooks.js
76–77	Regarding previous @inka comment: I'm checking if the message is the same because of this issue: https://linear.app/comm/issue/ENG-4101

I tested and now no threads are being fetched after the initial login

lib/hooks/message-hooks.js
63–65	what if fetchState is null?
76–77	So is this a hack? Should it be reverted when ENG-4101 is resolved? If so, please make it a subtask of ENG-4101