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

Rework api/user/repos for pagination #11827

Merged
merged 5 commits into from
Jun 13, 2020
Merged

Conversation

CirnoT
Copy link
Contributor

@CirnoT CirnoT commented Jun 10, 2020

  1. Add count to GetUserRepositories so that pagination can be supported for /user/{username}/repos
  2. Rework ListMyRepos to use models.SearchRepository

ListMyRepos was an odd one. It first fetched all user repositories and then tried to supplement them with accessible map. The end result was that:

  • Limit for pagination did not work because accessible repos would always be appended
  • The amount of pages was incorrect if one were to calculate it
  • When paginating, all accessible repos would be shown on every page

Hopefully it should now work properly. Fixes #11800 and does not require any change on Drone-side as it can properly interpret and act on Link header which we now set.

@CirnoT
Copy link
Contributor Author

CirnoT commented Jun 10, 2020

Needs to be backported to v1.12 as to not break Drone integration.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 10, 2020
routers/api/v1/user/repo.go Outdated Show resolved Hide resolved
routers/api/v1/user/repo.go Outdated Show resolved Hide resolved
routers/api/v1/user/repo.go Show resolved Hide resolved
@lafriks
Copy link
Member

lafriks commented Jun 10, 2020

lint error about unused "fmt" import

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jun 10, 2020
Copy link
Contributor

@zeripath zeripath left a comment

Choose a reason for hiding this comment

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

OK this has waited long enough for dronistas to pipe up and say this further breaks things. LGTM.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jun 13, 2020
@zeripath
Copy link
Contributor

make lg-tm work

@zeripath zeripath merged commit 0159851 into go-gitea:master Jun 13, 2020
@zeripath
Copy link
Contributor

@CirnoT please send backport

@CirnoT CirnoT deleted the api-myrepos branch June 13, 2020 11:53
@CirnoT CirnoT restored the api-myrepos branch June 13, 2020 11:53
@CirnoT CirnoT deleted the api-myrepos branch June 13, 2020 11:53
CirnoT added a commit to CirnoT/gitea that referenced this pull request Jun 13, 2020
* Add count to `GetUserRepositories` so that pagination can be supported for `/user/{username}/repos`
* Rework ListMyRepos to use models.SearchRepository

ListMyRepos was an odd one. It first fetched all user repositories and then tried to supplement them with accessible map. The end result was that:

* Limit for pagination did not work because accessible repos would always be appended
* The amount of pages was incorrect if one were to calculate it
* When paginating, all accessible repos would be shown on every page

Hopefully it should now work properly. Fixes go-gitea#11800 and does not require any change on Drone-side as it can properly interpret and act on Link header which we now set.

Co-authored-by: Lauris BH <[email protected]>
(cherry picked from commit 0159851)
@lafriks lafriks added the backport/done All backports for this PR have been created label Jun 13, 2020
zeripath pushed a commit that referenced this pull request Jun 13, 2020
* Add count to `GetUserRepositories` so that pagination can be supported for `/user/{username}/repos`
* Rework ListMyRepos to use models.SearchRepository

ListMyRepos was an odd one. It first fetched all user repositories and then tried to supplement them with accessible map. The end result was that:

* Limit for pagination did not work because accessible repos would always be appended
* The amount of pages was incorrect if one were to calculate it
* When paginating, all accessible repos would be shown on every page

Hopefully it should now work properly. Fixes #11800 and does not require any change on Drone-side as it can properly interpret and act on Link header which we now set.

Co-authored-by: Lauris BH <[email protected]>
(cherry picked from commit 0159851)
ydelafollye pushed a commit to ydelafollye/gitea that referenced this pull request Jul 31, 2020
* Add count to `GetUserRepositories` so that pagination can be supported for `/user/{username}/repos`
* Rework ListMyRepos to use models.SearchRepository

ListMyRepos was an odd one. It first fetched all user repositories and then tried to supplement them with accessible map. The end result was that:

* Limit for pagination did not work because accessible repos would always be appended
* The amount of pages was incorrect if one were to calculate it
* When paginating, all accessible repos would be shown on every page

Hopefully it should now work properly. Fixes go-gitea#11800 and does not require any change on Drone-side as it can properly interpret and act on Link header which we now set.

Co-authored-by: Lauris BH <[email protected]>
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] [API] make pagination optional for GET /api/user/repos
4 participants