copilot: add missing pagination to fetchOrganizationTeams

[christianherweg0807]

Hey, I just made a Pull Request!

This PR adds pagination to the CoPilot backend plugin.

resolves #2087

✔️ Checklist

• A changeset describing the change and affected packages. (more info)
• Added or updated documentation
• Tests for new functionality and regression tests for bug fixes
• Screenshots attached (for UI changes)
• All your commits have a Signed-off-by line in the message. (more info)

[backstage-goalie]

Changed Packages

Package Name	Package Path	Changeset Bump	Current Version
@backstage-community/plugin-copilot-backend	workspaces/copilot/plugins/copilot-backend	patch	v0.7.0

[backstage-goalie]

Thanks for the contribution!
All commits need to be DCO signed before they are reviewed. Please refer to the the DCO section in CONTRIBUTING.md or the DCO status for more info.

[awanlin]

Hi @christianherweg0807, if this is a POC then can you move this to be a Draft? That would have avoided reviewers being added. Is there something specific you need which has caused you to open the PR in this state instead of a reviewable contribution? How can the community help?

[henrikedegrd]

A suggestion would be to use octokit that have built-in support for pagination.

I am actually planning of doing that myself going forward with more rewrites to include more detailed statistics. But that might take a month or two.

[christianherweg0807]

@henrikedegrd
octokit would be the right tool, yeah. I didn´t want to introduce a new dependency for this plugin, just for this fix. I simplified the code. This should be ok for now. A rewrite would definitly use octokit.

Would be very interesting to have more detailed statistics. If there is a plugin or project to contribute ... maybe I could add some lines of code.

[christianherweg0807]

I don´t know how to resolve the "Missing Changesets" thing. Any help would be nice.

[henrikedegrd]

I don´t know how to resolve the "Missing Changesets" thing. Any help would be nice.

https://github.com/backstage/community-plugins/blob/main/CONTRIBUTING.md#creating-changesets

Essentially running yarn changeset in the plugin-folder and writing some information.

[henrikedegrd]

From my part, I would suggest replace all console.warn,error with correct calls to the logger.
That would have to be provided as a dependency to GithubClient from the router.

And probably remove all calls to debug.

[04kash]

Hi @christianherweg0807, thanks for taking this on!
In addition to the changes mentioned by @henrikedegrd (adding a changeset and using the LoggerService instead of console.log statements — there are a lot of plugins in this repo using it, let me know if you need examples), you'll also need to resolve the type errors causing the CI to fail.

[henrikedegrd]

Made some suggestions in order to simplify and provide the logger as mandatory.

[henrikedegrd]

I think it would be better to make the logger mandatory to provide.

You can provide it as a dependency here: https://github.com/backstage/community-plugins/blob/main/workspaces/copilot/plugins/copilot-backend/src/service/router.ts#L126

And it is availible here to grab: https://github.com/backstage/community-plugins/blob/main/workspaces/copilot/plugins/copilot-backend/src/service/router.ts#L122

[henrikedegrd]

This can possibly be simplified in order to reduce duplicated code.

Maybe the get() can use getRaw() since the only thing that differs is the return.

[04kash]

@henrikedegrd Thanks for the initial review! Once you've approved this PR, I'll proceed with my review. FYI @christianherweg0807, this is the process we'll follow for reviewing and merging this PR.

[henrikedegrd]

@04kash I have another PR #3241 which also fixes this issue (among also adding new functionality), however I fixed it by changing to octokit.

So this PR might be closed in favor of the octokit-solution?

[04kash]

So this PR might be closed in favor of the octokit-solution?

Sounds good to me! @christianherweg0807, does that work for you?

[04kash]

Closing in favour of #3241