Add some improvements to thread handling

[larkox]

Summary

The following improvements has been added:

• Default last_reply_at to the post create_at, if the thread is 0.
• To get the limits, consider the post create_at if the last_reply_at is 0
• Update the limits only if the received limits are smaller than the ones we have
• Add a real refresh (get the last 60) functionality to the pull to refresh on threads
• Always fetch the unreads when fetching threads (this may add stress to the server)
• Only show threads with replies

This should solve several issues, but mainly tackling two:

• Thread with replies and last_reply_at = 0 showing as 54 years old threads (and messing with the ordering)
• "In between unreads" (outside of the last fetched page) not showing in the unreads tab
• last_reply_at = 0 causing the "anti-gap" logic to fail
• Add a "quick fix" for latest threads (refresh)
• Threads without a reply showing in the thread list

This does NOT solve the following issues:

• Potential gaps caused by the thread api returning the threads based on membership update_at but not surfacing that information
• Refetching the same information over and over because the thread api returns the threads based on membership update_at but not surfacing that information
• Threads still appear in the list after unfollowing them while the app is closed

Some of the issues may need a database purge (logout and log back in) to get fixed.

Ticket Link

Fix https://mattermost.atlassian.net/browse/MM-61759
Fix https://mattermost.atlassian.net/browse/MM-60745

Release Note

Improve reliability of threads

[enahum]

Threads still appear in the list after unfollowing them while the app is closed.

This is kind of bad. Is that happening today?

Also, I believe there is some code doing something kind of the same in the notifications area.

Have we looked at how these thread gaps are different than fetching posts on a channel?

Will review soon. Just wanted to add this comment before I forget.

[larkox]

This is kind of bad. Is that happening today?

I would swear it does. I have not changed any logic around that.

Also, I believe there is some code doing something kind of the same in the notifications area.

I looked how we were adding the threads, and I was not much worried about the last_reply_at since I expected the threads to be fetch again when the app ran.
After looking it more carefully, I see that we are indeed updating the TeamThreadSync table on the native side, so that should be handled too. If this is the approach we want to take, I will try to update those.

Have we looked at how these thread gaps are different than fetching posts on a channel?

With fetching posts we have a more reliable update at (the post update at). Here we don't have such a thing right now (AFAIK). Also, since we are receiving "0 replies threads" (and therefore we have a last reply at as 0) that was messing with the bottom end of the gaps, something that would not happen with posts (the update_at is always above 0).

[devinbinnie]

A couple non-blocking questions otherwise LGTM.

[devinbinnie]

Do we not sync data on post edits? Or is that done per post?

[larkox]

The thread information in the database only includes the post id, so updates in the post should not affect this piece of data (it affects the post itself, which is done somewhere else).

[devinbinnie]

Just curious - where do we sync the DM/GM threads?

[larkox]

In other calls to syncTeamThreads, where we don't pass the excludeDirect option. Mainly on startup / reconnect, if I am not mistaken.

[enahum]

Exclude direct is false also for the initial team, this will fetch the threads on DM's and GM's that are returned for every team if you don't set the flag as true

[enahum]

This looks good to me, but I still think we should look into the notifications area

[larkox]

@enahum I added the changes to the native side.

It seems we had a bigger problem on android, since react native maps work as streams, so the moment you read from them, you cannot put more information. Therefore, we were never adding the threads to the notification data object. Copying to read is one option, but I can look for smarter solutions.

[enahum]

@enahum I added the changes to the native side.

It seems we had a bigger problem on android, since react native maps work as streams, so the moment you read from them, you cannot put more information. Therefore, we were never adding the threads to the notification data object. Copying to read is one option, but I can look for smarter solutions.

Can you expand on this? I don't quite understand what you mean

[larkox]

@enahum In order to create a map of values, we use the following:

val notificationData = Arguments.createMap()

Then, we add more values to it by calling stuff like:

notificationData.putMap("team", it)

And we get information out of the map with:

notificationData.getMap("myTeam")

So... this flow works fine:

val notificationData = Arguments.createMap()
notificationData.putMap("team", it)
notificationData.putMap("myTeam", it)
val team = notificationData.getMap("team")
val myTeam = notificationData.getMap("myTeam")

But this other one won't:

val notificationData = Arguments.createMap()
notificationData.putMap("team", it)
val team = notificationData.getMap("team")
notificationData.putMap("myTeam", it)
val myTeam = notificationData.getMap("myTeam")

With this flow, the myTeam key will never be added to the notificationData map, and the variable myTeam will be null.

From what I read, this is because of how these objects work on the inside, and the keyword I remember is that they "work like streams". So... one way to fix the flow I showed you before is copying the map before accessing its values:

val notificationData = Arguments.createMap()
notificationData.putMap("team", it)
val team = notificationData.copy().getMap("team")
notificationData.putMap("myTeam", it)
val myTeam = notificationData.getMap("myTeam")

[enahum]

@larkox got it, maybe then we should use hashmaps or bundles instead, thoughts?

[enahum]

Btw @larkox this PR now has conflict that needs resolution, perhaps that can be done while fixing the issue on Android native as well ?