Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

MM-17468 - Improving performance of fetching threads #3549

Merged
merged 5 commits into from
Sep 17, 2019

Conversation

reflog
Copy link
Contributor

@reflog reflog commented Aug 28, 2019

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

@reflog reflog changed the title added support for getting posts without fetching all thread posts MM-17468 - Improving performance of fetching threads Aug 28, 2019
@reflog reflog added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Aug 29, 2019
Copy link
Member

@jespino jespino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@lieut-data lieut-data left a 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?

@reflog
Copy link
Contributor Author

reflog commented Sep 2, 2019

@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

@lindalumitchell lindalumitchell added this to the v5.16.0 milestone Sep 3, 2019
@lindalumitchell lindalumitchell removed the 3: QA Review Requires review by a QA tester label Sep 3, 2019
@lieut-data lieut-data added 3: QA Review Requires review by a QA tester and removed 2: Dev Review Requires review by a core commiter labels Sep 5, 2019
@hanzei
Copy link
Contributor

hanzei commented Sep 6, 2019

@lindalumitchell Can you please confirm if this PR needs QA review?

@lindalumitchell
Copy link
Contributor

Thanks, this one is fine to continue. Removing QA Review label.

@lindalumitchell lindalumitchell removed the 3: QA Review Requires review by a QA tester label Sep 6, 2019
@hanzei hanzei added the 4: Reviews Complete All reviewers have approved the pull request label Sep 6, 2019
@hanzei
Copy link
Contributor

hanzei commented Sep 17, 2019

@reflog Is this ready to merge?

@reflog reflog merged commit c0f9cb8 into mattermost:master Sep 17, 2019
@reflog reflog deleted the MM-17468 branch September 17, 2019 16:15
@amyblais amyblais added Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation labels Sep 17, 2019
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
@lindy65 lindy65 modified the milestones: v5.16.0 , v5.18.0 Sep 30, 2019
lieut-data added a commit that referenced this pull request Sep 30, 2019
* Revert "Automated cherry pick of #3776 (#3777)"

This reverts commit 647eba0.

* Revert "MM-17468 - Improving performance of fetching threads (#3549)"

This reverts commit c0f9cb8.

* temporary redux commit for testing

* update redux commit
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
@lindy65 lindy65 added the Tests/Not Needed Does not require new release tests label Nov 25, 2019
Willyfrog added a commit to Willyfrog/mattermost-webapp that referenced this pull request Nov 27, 2019
reflog added a commit to reflog/mattermost-webapp that referenced this pull request Jan 13, 2020
reflog added a commit that referenced this pull request Jan 15, 2020
* Revert "MM-18623  - Handle reply-count in getPostThread code path (#3776)"

This reverts commit 4d7a834.

* Revert "MM-17468 - Improving performance of fetching threads (#3549)"

This reverts commit c0f9cb8.

* typo

* redux bump
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
Projects
None yet
7 participants