added AdminListOrgsOptions #202

spawn2kill · 2019-12-19T22:57:15Z

spawn2kill commented

2019-12-19 22:57:15 +00:00

Since AdminListOrgs function is limited to 10 results, it is currently impossible to get all admin's organizations.

This PR creates a AdminListOrgsOptions struct which allows to use Gitea's API URL query parameters in order to increase the limit of results or by getting another page of results.

Since `AdminListOrgs` [function](https://gitea.com/gitea/go-sdk/src/commit/b374d7ccc4b455f1e1829ab3ff0808de80bc39e0/gitea/admin_org.go#L14-L18) is limited to 10 results, it is currently impossible to get **all** admin's organizations. This PR creates a `AdminListOrgsOptions` struct which allows to use Gitea's API URL query parameters in order to increase the limit of results or by getting another page of results.

6543 reviewed 2019-12-20 00:10:55 +00:00

Dismissed

gitea/admin_org.go

						
				@ -12,2 +12,4 @@

				)

				// AdminListOrgsOptions options for listing admin's organizations

				type AdminListOrgsOptions struct {

6543 commented

2019-12-20 00:10:55 +00:00

this look like a struct whitch could be used in general

is there already a struct named like paginationOptions or so?

this look like a struct whitch could be used in general is there already a struct named like **paginationOptions** or so?

spawn2kill commented

2019-12-20 00:20:44 +00:00

Honestly I don't know the Gitea API so that well, just ran into that issue, fixed it and created a PR for it.

For what I could see rn, there are some structs defining the same URL params for different requests like this

Anyway, that implementation is not right since the slice is being created with a fixed size of 10... If I change the limit of the options to let's say 30, it will still be limited by the slice length.

There are some other implementations like this which by the way are not used by the Gitea API

I suggest a refactor in all SDK requests with the correct implementation of the Gitea API... but for now I guess this PR fixes the problem for this request.

I might work on that refactor as soon as I have some free time for that

Honestly I don't know the Gitea API so that well, just ran into that issue, fixed it and created a PR for it. For what I could see rn, there are some structs defining the same URL params for different requests like [this](https://gitea.com/gitea/go-sdk/src/commit/b374d7ccc4b455f1e1829ab3ff0808de80bc39e0/gitea/issue.go#L46) Anyway, that implementation is not right since the slice is being created with a fixed size of 10... If I change the limit of the options to let's say 30, it will still be limited by the slice length. There are some other implementations like [this](https://gitea.com/gitea/go-sdk/src/commit/b374d7ccc4b455f1e1829ab3ff0808de80bc39e0/gitea/user_follow.go#L10) which by the way are not used by the Gitea [API](https://try.gitea.io/api/swagger#/user/userCurrentListFollowing) I suggest a refactor in all SDK requests with the correct implementation of the Gitea API... but for now I guess this PR fixes the problem for this request. I might work on that refactor as soon as I have some free time for that

lunny commented

2019-12-20 00:37:45 +00:00

I think we can follow some rules of https://github.com/google/go-github.
We can have type ListOptions for a common parameters.

type ListOptions struct {
	Page int
	PerPage int
}

And then we can create a OrgListOptions which include ListOptions. It could be used by all function to retrieve organization list.

type OrgListOptions struct {
    ListOptions
}

I think we can follow some rules of https://github.com/google/go-github. We can have `type ListOptions` for a common parameters. ```go type ListOptions struct { Page int PerPage int } ``` And then we can create a `OrgListOptions` which include `ListOptions`. It could be used by all function to retrieve organization list. ```go type OrgListOptions struct { ListOptions } ```

spawn2kill commented

2019-12-20 00:49:44 +00:00

I do agree with you @lunny but that change only makes sense if Gitea's API is also refactored in order to support pagination in all GET /list requests... The API itself needs to be uniformed before uniforming the SDK. I mean, why does this request takes pagination, but this and this doesn't?

I do agree with you @lunny but that change only makes sense if Gitea's API is also refactored in order to support pagination in all GET /list requests... The API itself needs to be uniformed before uniforming the SDK. I mean, why does [this](https://try.gitea.io/api/swagger#/admin/adminGetAllOrgs) request takes pagination, but [this](https://try.gitea.io/api/swagger#/admin/adminGetAllUsers) and [this](https://try.gitea.io/api/swagger#/organization/orgListMembers) doesn't?

lunny commented

2019-12-20 00:55:20 +00:00

Right. We need more work to do on APIs. :(

spawn2kill commented

2019-12-20 01:02:04 +00:00

I can see there are some open issues regarding the pagination problem on the API like:
https://github.com/go-gitea/gitea/issues/8131
https://github.com/go-gitea/gitea/issues/8127
https://github.com/go-gitea/gitea/issues/5632

Maybe we can define that all routes that returns a list of anything will support pagination in the future with page and limit URL parameters, limit field being limited to 50. If something like that gets defined, I can improve this PR in order to add that future support for pagination.

I can see there are some open issues regarding the pagination problem on the API like: https://github.com/go-gitea/gitea/issues/8131 https://github.com/go-gitea/gitea/issues/8127 https://github.com/go-gitea/gitea/issues/5632 Maybe we can define that all routes that returns a list of anything will support pagination in the future with `page` and `limit` URL parameters, `limit` field being limited to 50. If something like that gets defined, I can improve this PR in order to add that future support for pagination.

spawn2kill commented

2019-12-20 01:08:08 +00:00

There is some work done about this uniformity like here. That logic will only have to be replicated across all APIs.

There is some work done about this uniformity like [here](https://github.com/go-gitea/gitea/blob/master/routers/api/v1/admin/org.go#L102). That logic will only have to be replicated across all APIs.

6543 commented

2019-12-20 02:22:14 +00:00

@spawn2kill I like lunny's idear: #202 (comment) can you change your PR in this direction

and sure API need some refactor - to ad a new query is not that hard because it is backwards compatible :D

@spawn2kill I like lunny's idear: https://gitea.com/gitea/go-sdk/pulls/202#issuecomment-104096 can you change your PR in this direction and sure API need some refactor - to ad a new query is not that hard because it is backwards compatible :D

spawn2kill closed this pull request

2019-12-20 12:08:51 +00:00

spawn2kill referenced this pull request

2019-12-20 12:17:22 +00:00

added pagination options for listing requests #203

6543 added this to the v0.11.0 milestone 2019-12-30 20:42:46 +00:00

continuous-integration/drone/pr Build is passing

Details

testing / testing*

Required

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

No reviewers

No Milestone

No Assignees

3 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: gitea/go-sdk#202