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

always set the number on pagination param when fetching chat threads CORE-9096 #1839

Merged
merged 3 commits into from Oct 4, 2018

Conversation

Projects
None yet
2 participants
@mmaxim
Member

mmaxim commented Oct 4, 2018

Otherwise it would send 0 over on the subsequent pages and get 0 results.

Also, do we really need the entire thread? Can we check in this loop to see if we have enough data to reasonably render the widget? It seems like we can abort out of this loop and save a bunch of traffic.

@mmaxim mmaxim requested a review from strib Oct 4, 2018

@mmaxim

This comment has been minimized.

Show comment
Hide comment
@mmaxim

mmaxim Oct 4, 2018

Member

Oh wait nvm, I see how it works, it does break out early.

Member

mmaxim commented Oct 4, 2018

Oh wait nvm, I see how it works, it does break out early.

@strib

strib approved these changes Oct 4, 2018

Thanks, sorry for the confusion. Looks good mod a non-broken CI runner.

@mmaxim mmaxim merged commit d26d2b1 into master Oct 4, 2018

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/jenkins/pr-head This commit looks good
Details

@mmaxim mmaxim deleted the mike/CORE-9096 branch Oct 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment