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

Return a UserList from /api/v1/admin/users #6629

Merged
merged 7 commits into from
Apr 15, 2019

Conversation

zeripath
Copy link
Contributor

The new /api/v1/admin/users endpoint doesn't return a UserList as described on swagger and doesn't return api.User but rather an unsanitised form of models.User.

This PR fixes this.

@zeripath zeripath added backport/v1.8 modifies/api This PR adds API routes or modifies them type/bug labels Apr 15, 2019
@zeripath zeripath added this to the 1.9.0 milestone Apr 15, 2019
@lunny
Copy link
Member

lunny commented Apr 15, 2019

How about make a function convertUser to convert a models.User to api.User so that two places could use it?

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 15, 2019
Signed-off-by: Andrew Thornton <[email protected]>
@lunny
Copy link
Member

lunny commented Apr 15, 2019

And any test for this API is better!

@codecov-io
Copy link

codecov-io commented Apr 15, 2019

Codecov Report

Merging #6629 into master will increase coverage by 0.05%.
The diff coverage is 89.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6629      +/-   ##
==========================================
+ Coverage   40.47%   40.52%   +0.05%     
==========================================
  Files         406      406              
  Lines       54484    54492       +8     
==========================================
+ Hits        22052    22084      +32     
+ Misses      29395    29372      -23     
+ Partials     3037     3036       -1
Impacted Files Coverage Δ
routers/api/v1/user/user.go 25.19% <0%> (+1.49%) ⬆️
routers/api/v1/convert/convert.go 80.68% <100%> (+1.41%) ⬆️
routers/api/v1/admin/user.go 29.24% <83.33%> (+7.26%) ⬆️
modules/log/event.go 64.46% <0%> (-1.53%) ⬇️
routers/repo/view.go 41.08% <0%> (-1%) ⬇️
models/user.go 50.32% <0%> (+0.55%) ⬆️

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 74fc636...8c89537. 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 Apr 15, 2019
@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 Apr 15, 2019
@lunny
Copy link
Member

lunny commented Apr 15, 2019

But CI build failed.

@zeripath
Copy link
Contributor Author

Sigh sorry about that.

@techknowlogick techknowlogick merged commit 8371168 into go-gitea:master Apr 15, 2019
@techknowlogick
Copy link
Member

@zeripath merged 😃 please backport.

@zeripath zeripath deleted the fix-api-admin-users branch April 15, 2019 20:16
zeripath added a commit to zeripath/gitea that referenced this pull request Apr 15, 2019
@lafriks lafriks added the backport/done All backports for this PR have been created label Apr 17, 2019
@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. modifies/api This PR adds API routes or modifies them type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants