This repository has been archived by the owner on Mar 13, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
MM-17468 - Improving performance of fetching threads #3549
Merged
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
This was referenced Aug 28, 2019
jespino
approved these changes
Aug 29, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
lieut-data
suggested changes
Aug 30, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of high-level questions:
- What guarantees that
post.reply_thread_count
stays up-to-date? Previously, we could rely on incoming posts in the Redux state, but presumably that won't impact this new bit? - What now triggers loading the RHS on demand? Does that happen every time the RHS is opened?
- What's the loading experience look like on the RHS until we fetch the thread?
@lieut-data - reworked the PR, now that the server always returns the root post as well with the proper count - results should be displayed properly |
lieut-data
approved these changes
Sep 5, 2019
@lindalumitchell Can you please confirm if this PR needs QA review? |
Thanks, this one is fine to continue. Removing QA Review label. |
lindalumitchell
approved these changes
Sep 6, 2019
@reflog Is this ready to merge? |
jwilander
pushed a commit
that referenced
this pull request
Sep 24, 2019
* added support for getting posts without fetching all thread posts * test fix * minor fixes
lieut-data
added a commit
that referenced
this pull request
Sep 27, 2019
lieut-data
added a commit
that referenced
this pull request
Sep 30, 2019
skheria
pushed a commit
to uber-archive/mattermost-webapp
that referenced
this pull request
Oct 3, 2019
* added support for getting posts without fetching all thread posts * test fix * minor fixes
skheria
pushed a commit
to uber-archive/mattermost-webapp
that referenced
this pull request
Oct 3, 2019
* added support for getting posts without fetching all thread posts * test fix * minor fixes
Willyfrog
added a commit
to Willyfrog/mattermost-webapp
that referenced
this pull request
Nov 27, 2019
reflog
added a commit
that referenced
this pull request
Jan 15, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
4: Reviews Complete
All reviewers have approved the pull request
Changelog/Done
Required changelog entry has been written
Docs/Not Needed
Does not require documentation
Tests/Not Needed
Does not require new release tests
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
Added support for new 'response_count' field in Post to improve performance for threaded post fetching.
Ticket Link
Fixes https://mattermost.atlassian.net/browse/MM-17468
Related Pull Requests
mattermost/mattermost#11980
mattermost/mattermost-redux#911