Fix Loading of Older Thread Messages in Thread Message Modal

[smritidoneria]

Brief Title

This pull request addresses the issue where older thread messages are not being loaded in the thread message modal.

Acceptance Criteria fulfillment

• Updated the getAllThreadMessages function in EmbeddedChatApi.ts to correctly fetch older thread messages. -Updated the ChatBody component to correctly use the getAllThreadMessages function from the useFetchChatData hook and update the state.

Fixes #895

Video/Screenshots

https://github.com/user-attachments/assets/f2912771-dd29-4320-b59f-e5f13a347169

PR Test Details

Note: The PR will be ready for live testing at https://rocketchat.github.io/EmbeddedChat/pulls/pr-<pr_number> after approval. Contributors are requested to replace <pr_number> with the actual PR number.

[abirc8010]

Hey @smritidoneria, I think fetching threads only through the API once on opening the sidebar might not retrieve older messages due to the limit on the number of items it returns. Perhaps we could consider implementing a "load more threads" (by setting required offset) feature on scroll down to handle this

[smritidoneria]

hey @abirc8010 , Thanks for pointing out, I will look after this.

[abirc8010]

Hi @smritidoneria, since this issue affects not only threads but also other items in the sidebar, it might be better to address the loading of more items in the sidebar in a single PR. You can refer to a similar issue #915 for reference.

[smritidoneria]

I was reviewing the pr and find out some issue the query parameters are not passing in the url correctly except for the roomID refrence-

Calling the function like this

but the url shows

and, where can I see the logs for the file EmbeddedChatApi.ts. it is neither showing in browser tools nor in terminal?

Any suggestions @abirc8010 , @Spiral-Memory , @SinghaAnirban005

[abirc8010]

and, where can I see the logs for the file EmbeddedChatApi.ts. it is neither showing in browser tools nor in terminal?

Hey @smritidoneria you can find the logs in the browser console, did you run yarn build in the api directory after your changes?

Also I found this in ChatLayout.js , try to import the getAllThreadMessages function from useFetchchatData() and call it inside the useEffect

Btw is there any reason you’re passing rid? You can append it to the request URL as this.rid.

[SinghaAnirban005]

@smritidoneria you probably missed out on building the api package , Also take care of the pagination params like offset and count

@abirc8010 i believe doing that useEffect thing in ChatLayout.js is causing the threads endpoint to be called many times such as closing of sidebar and opening of sidebar for pinned or starred messages. Thus we must avoid calling it there

[smritidoneria]

@smritidoneria you probably missed out on building the api package , Also take care of the pagination params like offset and count

@abirc8010 i believe doing that useEffect thing in ChatLayout.js is causing the threads endpoint to be called many times such as closing of sidebar and opening of sidebar for pinned or starred messages. Thus we must avoid calling it there

yaa, actually I forgot to build the package, thanks!

[smritidoneria]

hey @abirc8010 , @SinghaAnirban005 , I'm encountering an issue with the scroll event not being detected in the message aggregator component,. I have tried using many ways , I have tried giving ref to the threaded message main container that didn't work, and then the message aggregator component main box, which is also not working kindly ignore the rest of the code, it needs to be optimized

suggestions?

[abirc8010]

Hey @smritidoneria instead of maintaining a ref could we go with this approach ?

Additionally, I think the logic in the MessageAggregator component needs to be refactored to make it more extensible, currently it fetches all the messages from the messageStore then filters messages based on shoudRender , maybe we can pass only the required filtered messages as a prop to MessageAggregator

The functionality needs to be extendable so that a similar logic can be applied to rest of the MessageAggregators like starred or pinned

[smritidoneria]

Yes, @abirc8010, we can proceed with the approach you mentioned. However, in the main ChatInterface, we are currently addressing this issue using ref. To maintain the application's consistency and serializability, we should first attempt to resolve this using ref. If we are unable to do so after further effort, we can then switch to your proposed approach.

[Spiral-Memory]

Hey all,

Really busy these days.. and not getting the time to review.. will review as soon as i get some time

Thanks

[smritidoneria]

Hey everyone,

With the merge of the PR that enables loading older messages when reaching the top view, this issue is automatically resolved. Once the previous messages are loaded into the DOM view, the thread messages are fetched and displayed accordingly.

I don't believe any further changes are needed. What do you think, @Spiral-Memory, @abirc8010, @SinghaAnirban005?

Reference-

https://github.com/user-attachments/assets/d2b88238-cb23-4be4-831a-868856849744