Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MM-17044 Load posts until no more are needed #3656

Merged
merged 3 commits into from Jan 7, 2020
Merged

MM-17044 Load posts until no more are needed #3656

merged 3 commits into from Jan 7, 2020

Conversation

enahum
Copy link
Contributor

@enahum enahum commented Dec 3, 2019

Summary

When the post list updates and there are more posts to load and the screen has not been filled yet we will continue on loading posts until one of the conditions are met, this helps avoiding a continuous loading indicator when there are too many combined system messages in a channel.

Ticket Link

https://mattermost.atlassian.net/browse/MM-17044

@enahum enahum added 2: Dev Review Requires review by a core commiter CherryPick/Approved Meant for the quality or patch release tracked in the milestone 3: QA Review Requires review by a QA tester labels Dec 3, 2019
@enahum enahum added this to the v1.27 milestone Dec 3, 2019
Copy link
Member

@saturninoabril saturninoabril left a comment

Choose a reason for hiding this comment

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

Tested and passed, except for one request.

app/components/post_list/post_list.test.js Outdated Show resolved Hide resolved
Copy link
Member

@saturninoabril saturninoabril left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM.

@migbot migbot removed the 2: Dev Review Requires review by a core commiter label Dec 3, 2019
@DHaussermann
Copy link

@enahum can you clarify if this is for 1.26 or 1.27? The linked ticket says 1.27 but I'm confused by the CherryPick Approved label. Maybe the mobile branching is unclear to me.

@enahum
Copy link
Contributor Author

enahum commented Dec 12, 2019

@DHaussermann the mobile branching is exactly the same as the rest. Basically this will be cherry picked for 1.27 once the branch is created.

@amyblais amyblais removed the CherryPick/Approved Meant for the quality or patch release tracked in the milestone label Jan 3, 2020
@amyblais amyblais removed this from the v1.27 milestone Jan 3, 2020
@migbot
Copy link
Contributor

migbot commented Jan 3, 2020

/update-branch

Copy link

@DHaussermann DHaussermann left a comment

Choose a reason for hiding this comment

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

Tested and passed

  • Issue is resolved. The app will fetch all join/leave posts
  • Exploratory testing did not find any cases of missing posts
  • Performed tests where some posts where made and deleted while the user was not logged in. All correct posts are shown.
    LGTM!

@DHaussermann DHaussermann added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Jan 6, 2020
@amyblais amyblais added this to the v1.28 milestone Jan 7, 2020
@DHaussermann
Copy link

@migbot or @enahum Can we have this merged to ensure the next RN build includes it.

@migbot migbot merged commit 5d74300 into master Jan 7, 2020
@migbot migbot deleted the MM-17044 branch January 7, 2020 18:59
@DHaussermann
Copy link

DHaussermann commented Jan 7, 2020

@migbot sorry, I missed that Amy added the v1.28 Not sure now if this should be merged.

@migbot
Copy link
Contributor

migbot commented Jan 7, 2020

@DHaussermann Shouldn't be a problem as v1.28 will be cut from master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants