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

Feature - Pagination for git tree API #5838

Merged

Conversation

richmahn
Copy link
Contributor

@richmahn richmahn commented Jan 25, 2019

Issue #5837

This PR adds pagination to the repos/{owner}/{repo}/git/trees/{sha} API to not have the restriction Github has truncating to the first 1000 entries and not show any more (GitHub docs say to see all files of big repos you'll have to clone the repo, yet we are trying to avoid that in our mobile app).

Now if you add ?page=2 (page is 1 when not given) as a query parameter it will show the next page. If there are still more items after the specified page, truncated will remain as true, when it is the last page, truncated will be false.

Also adds per_page= where per_page is how many items to show per page. Default is 1000 and is settable in the app.ini file as DFAULT_GIT_TREES_PER_PAGE

This PR also adds to the API docs the recursive=1 and this page=# query parameters for this API call.

Testing:
Can be tested with a repo with lots of files, such as https://git.door43.org/unfoldingWord/en_ugl/ and then once ported into a repo fo the same name of your localhost root user, curl --request GET --url "http://localhost:3000/api/v1/repos/root/en_ugl/git/trees/master?recursive=1&page=2" for the 2nd page (curl --request GET --url "http://localhost:3000/api/v1/repos/root/en_ugl/git/trees/master?recursive=1&page=30" for the last page, and if you do page 31 you should get a tree that is null)

@lunny lunny added the type/enhancement An improvement of existing functionality label Jan 25, 2019
@techknowlogick
Copy link
Member

Please run make fmt and commit changes.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 25, 2019
@techknowlogick
Copy link
Member

techknowlogick commented Jan 25, 2019

Please add per_page param as well, see #5831 for an example

@techknowlogick techknowlogick added this to the 1.8.0 milestone Jan 25, 2019
@richmahn
Copy link
Contributor Author

@techknowlogick Both your requests done. While I know Chinese, I didn't update the Chinese version of the cheat-sheet like the example you gave me. Hopefully picked an ok name for this additional paging num setting but made it different to not be confused with the one from your example.

@lunny
Copy link
Member

lunny commented Jan 25, 2019

@richmahn I think we should start page from 1 not 0 since every other place did that.

@lunny
Copy link
Member

lunny commented Jan 25, 2019

And the CI failed.

@lafriks
Copy link
Member

lafriks commented Jan 25, 2019

To fix CI run:

make generate-swagger

@richmahn
Copy link
Contributor Author

@lunny ok, fixed and fixed.

@codecov-io
Copy link

codecov-io commented Jan 25, 2019

Codecov Report

Merging #5838 into master will increase coverage by 0.08%.
The diff coverage is 45.83%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5838      +/-   ##
==========================================
+ Coverage   38.71%   38.79%   +0.08%     
==========================================
  Files         330      330              
  Lines       48697    48729      +32     
==========================================
+ Hits        18853    18906      +53     
+ Misses      27115    27083      -32     
- Partials     2729     2740      +11
Impacted Files Coverage Δ
modules/setting/setting.go 51.77% <ø> (ø) ⬆️
routers/api/v1/repo/tree.go 41.12% <45.83%> (+41.12%) ⬆️
models/unit.go 14.28% <0%> (+14.28%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c840a9...64b3858. Read the comment docs.

@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 Jan 25, 2019
@techknowlogick
Copy link
Member

@richmahn to update vendor Gopkg.toml (and related files) the command to run is dep ensure -update code.gitea.io/sdk

templates/swagger/v1_json.tmpl Outdated Show resolved Hide resolved
@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 Feb 6, 2019
@lafriks
Copy link
Member

lafriks commented Feb 6, 2019

@zeripath need your review

@zeripath zeripath merged commit da1edbf into go-gitea:master Feb 6, 2019
richmahn added a commit to richmahn/gitea that referenced this pull request Apr 11, 2019
* Feature - Pagination for git tree API

* Handles case when page is negative

* Does a for loop over the start and end rather than all entries

* Removed redundent logic

* Adds per_page as a query parameter

* Adds DEFAULT_GIT_TREES_PER_PAGE for settings, ran make fmt

* Fix typo in cheat-sheet en

* Makes page start at 1, generated swagger

* Use updates to SDK

* Updates to use latest sdk

* Updates swagger for tree api

* Adds test for GetTreeBySHA

* Updates per PR reviews

* Updates per PR reviews

* Remove file

* Formatting

* Fix to swagger file

* Fix to swagger

* Update v1_json.tmpl

* Fix to swagger file
@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
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants