Page MenuHomePhabricator

[native] Fetch latest messages in chat list
AbandonedPublic

Authored by michal on Jun 2 2023, 6:17 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Mar 4, 11:27 PM
Unknown Object (File)
Mon, Mar 4, 11:27 PM
Unknown Object (File)
Mon, Mar 4, 11:27 PM
Unknown Object (File)
Mon, Mar 4, 11:27 PM
Unknown Object (File)
Feb 22 2024, 11:12 PM
Unknown Object (File)
Feb 22 2024, 8:59 PM
Unknown Object (File)
Feb 22 2024, 8:58 PM
Unknown Object (File)
Feb 22 2024, 7:36 PM
Subscribers

Details

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

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!
michal requested review of this revision.Jun 2 2023, 8:59 AM
inka requested changes to this revision.Jun 5 2023, 5:26 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

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?

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)

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"

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

inka requested changes to this revision.Jun 19 2023, 2:40 AM

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

This revision now requires changes to proceed.Jun 19 2023, 2:40 AM

We are changing the approach for this task, so I'm abandoning this diff. More info in the linear task.