This repository has been archived by the owner on Mar 13, 2024. It is now read-only.
MM-39073: fixes threads unread tab #9047
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Clicking on an unread thread sometimes made the thread disappear. This happened because the getThread selector was looking in both all threads and in unread threads to verify the thread exists in this team, but if you landed at unread threads tab after a refresh you don't have more than 5 threads in all threads reducer, and if you click a thread it was getting removed from the unread reducer. This commit fixes that by refactoring the getThread selector to use user's current team channels to check if threads belong in a current team's channel. There is still a way to have that buggy behavior even after this fix. The scenario is if you have left a channel you don't get the threads removed as of yet, so you could replicate the above buggy behavior in that case. Since we are going to fix this in a later task (threads will be removed from "left" channels) I didn't bother to fix it as well.
koox00
added
2: Dev Review
koox00
added
the
Setup Cloud Test Server
|
No Kubernetes clusters available at the moment, please contact the Mattermost Cloud Team or wait a bit. |
mm-cloud-bot
removed
the
Setup Cloud Test Server
koox00
added
the
Setup Cloud Test Server
hmhealey
suggested changes
Oct 5, 2021
| export const getThread: (state: GlobalState, threadId: $ID<UserThread> | undefined) => UserThread | null = createSelector( | ||
| 'getThread', | ||
| getThreads, | ||
| (state: GlobalState) => getMyChannels(state).map((c) => c.id), |
There was a problem hiding this comment.
This would also need its own memoization since it's returning a new array of channel IDs each time this is called, even if the state hasn't changed.
|
@hmhealey, @jgilliam17 can you prioritize reviewing this PR? It's needed for this one as well #9082 |
hmhealey
approved these changes
Oct 7, 2021
jgilliam17
approved these changes
Oct 7, 2021
There was a problem hiding this comment.
Thanks @koox00
Tested, looks good to merge.
- Verified opening older unread threads after refresh is working as expected; opening an unread thread that is lower on the unread tab list than top 5 threads - no longer disappear.
jgilliam17
added
4: Reviews Complete
|
Test server destroyed |
amyblais
added
Changelog/Not Needed
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Clicking on an unread thread sometimes made the thread disappear.
This happened because the
getThreadselector was looking in both allthreads and in unread threads to verify the thread exists in this team,
but if you landed at unread threads tab after a refresh you don't have
more than 5 threads in all threads reducer, and if you click a thread it
was getting removed from the unread reducer.
This commit fixes that by refactoring the
getThreadselector to usegetMyChannelsselector to check if threads belong in a currentteam's channel.
PS1: The number 5 is not random, we fetch 5 threads (no unread filter) at mount
so we get the counts.
PS2: There is still a way to have that buggy behavior even after this fix.
The scenario is if you have left a channel you don't get the threads
removed as of yet, so you could replicate the above buggy behavior in
that case.
Since we are going to fix this in a later task (threads will be removed
from "left" channels) I didn't bother to fix it as well.
Ticket Link
https://mattermost.atlassian.net/browse/MM-39073
Release Note