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

fix: Added cursor pagination to slackapp conversations query #4442

Merged
merged 1 commit into from
Aug 8, 2023

Conversation

chriswk
Copy link
Contributor

@chriswk chriswk commented Aug 8, 2023

We've been struggling with not getting hold of all channels. Reading the documentation I found the support for next_cursor in the response_metadata, as long as the response has a next_cursor, there are responses left, so this PR adds a loop for querying until the next cursor or the channels list is undefined or ''. This should solve the issue Wayfair had as well.

@chriswk chriswk self-assigned this Aug 8, 2023
@sonatype-lift
Copy link

sonatype-lift bot commented Aug 8, 2023

Sonatype Lift is retiring

Sonatype Lift will be retiring on Sep 12, 2023, with its analysis stopping on Aug 12, 2023. We understand that this news may come as a disappointment, and Sonatype is committed to helping you transition off it seamlessly. If you’d like to retain your data, please export your issues from the web console.
We are extremely grateful and thank you for your support over the years.

📖 Read about the impacts and timeline

@vercel
Copy link

vercel bot commented Aug 8, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
unleash-docs ⬜️ Ignored (Inspect) Visit Preview Aug 8, 2023 9:22am
unleash-monorepo-frontend ⬜️ Ignored (Inspect) Visit Preview Aug 8, 2023 9:22am

}
nextCursor = moreChannels.response_metadata?.next_cursor;
this.logger.debug(
`This page had ${channels.length} channels`,
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe introducting a metric of number of slack channels found could be helpfull?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, separate PR.

Copy link
Contributor

@gardleopard gardleopard left a comment

Choose a reason for hiding this comment

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

Good stuff!

Copy link
Member

@nunogois nunogois left a comment

Choose a reason for hiding this comment

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

LGTM

@chriswk chriswk merged commit edcbf2a into main Aug 8, 2023
11 checks passed
@chriswk chriswk deleted the task/slackAppPagination branch August 8, 2023 10:54
gastonfournier pushed a commit that referenced this pull request Aug 11, 2023
We've been struggling with not getting hold of all channels. Reading the
documentation I found the support for next_cursor in the
response_metadata, as long as the response has a next_cursor, there are
responses left, so this PR adds a loop for querying until the next
cursor or the channels list is undefined or `''`. This should solve the
issue Wayfair had as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants