Skip to content

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.

Signed-off-by: Andrew Thornton <art27@cantab.net>
@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 <art27@cantab.net>
@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