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.

Sign up for GitHub

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

Jump to bottom

Iterate through pages of Bitbucket API's response #8891

Merged
merged 1 commit into from
Mar 29, 2022
Merged

Iterate through pages of Bitbucket API's response #8891

merged 1 commit into from
Mar 29, 2022

Conversation

laushinka
Copy link
Contributor

@laushinka laushinka commented Mar 18, 2022 •
edited by jldec
Loading

Description

This change allows iterating through all the pages of Bitbucket API's response.

Related Issue(s)

Fixes #8650

How to test

  1. Have some BitBucket repositories.
  2. Go to https://laushinka-0486385fa8.staging.gitpod-dev.com/new
  3. See all repositories

Release Notes

Allows fetching more than 100 BitBucket repositories.

Documentation

@laushinka laushinka requested a review from a team March 18, 2022 19:07
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Mar 18, 2022
@laushinka laushinka requested review from jldec and AlexTugarev March 18, 2022 19:08
@laushinka laushinka marked this pull request as draft March 18, 2022 19:12
@laushinka laushinka force-pushed the laushinka/cannot-see-all-our-repositories-8650 branch from a5d6255 to 6a402ed Compare March 18, 2022 19:26
jldec
jldec reviewed Mar 18, 2022
View reviewed changes
components/server/ee/src/bitbucket/bitbucket-app-support.ts
.list({
workspace,
pagelen: 100,
pagelen,
page,
role: "admin", // installation of webhooks is allowed for admins only
})
.catch((e) => {
console.error(e);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a simple way to show errors like this to users without requiring user interaction like alert() or confirm() do? I like the pattern in GitHub where it adds a blue banner at the top with error or success messages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you have a screenshot of an example? Either on our own platform or on GitHub.

Copy link
Member

Choose a reason for hiding this comment

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

That's a nice idea. I think we were discussing this already and the toast notification was included as well.
cc. @gtsiolis

Copy link
Contributor

Choose a reason for hiding this comment

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

I have opened separate issue #8933.
Keen to ship this fix.

@laushinka laushinka force-pushed the laushinka/cannot-see-all-our-repositories-8650 branch from 6a402ed to f6bcb39 Compare March 21, 2022 07:29
@AlexTugarev
Copy link
Member

@laushinka if you want to have this tested in a sustainable way, I'd suggest to make the pagelen configurable, e.g. moving the code to a function with such a param. This way we could add a "unit test" which would not require you to create 100 repos to test with ;-)

@laushinka laushinka force-pushed the laushinka/cannot-see-all-our-repositories-8650 branch from f6bcb39 to 5f860ae Compare March 23, 2022 16:57
@laushinka laushinka marked this pull request as ready for review March 25, 2022 07:33
@laushinka laushinka requested review from AlexTugarev and jldec March 25, 2022 07:33
@laushinka laushinka force-pushed the laushinka/cannot-see-all-our-repositories-8650 branch from 5f860ae to f781069 Compare March 28, 2022 11:38
@jldec
Copy link
Contributor

jldec commented Mar 29, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-laushinka-cannot-see-all-our-repositories-865.7

@jldec
Copy link
Contributor

jldec commented Mar 29, 2022

/hold (to reset pagelen)

jldec
jldec previously approved these changes Mar 29, 2022
View reviewed changes
Copy link
Contributor

@jldec jldec left a comment

Choose a reason for hiding this comment

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

works for me in testing - thanks @laushinka

@laushinka laushinka force-pushed the laushinka/cannot-see-all-our-repositories-8650 branch from f20e826 to 34d9042 Compare March 29, 2022 07:43
@laushinka
Copy link
Contributor Author

/hold (to reset pagelen)

Pagelength reset, will /unhold

@laushinka laushinka requested a review from jldec March 29, 2022 08:03
jldec
jldec approved these changes Mar 29, 2022
View reviewed changes
Copy link
Contributor

@jldec jldec left a comment

Choose a reason for hiding this comment

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

LGTM

@roboquat roboquat merged commit 106c508 into main Mar 29, 2022
@roboquat roboquat deleted the laushinka/cannot-see-all-our-repositories-8650 branch March 29, 2022 14:02
@jankeromnes jankeromnes changed the title Iterate through pages Iterate through pages of Bitbucket API's response Mar 31, 2022
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Mar 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jldec jldec jldec approved these changes

@AlexTugarev AlexTugarev Awaiting requested review from AlexTugarev

Assignees
No one assigned
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note size/M team: webapp Issue belongs to the WebApp team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cannot see all our repositories with BitBucket (Pagination issue?)
4 participants
@laushinka @AlexTugarev @jldec @roboquat