Skip to content

Make /users/search return a UserList #4846

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

Closed

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented Sep 2, 2018

As per issue #4841 . The current implementation of the /users/search API does not return a UserList but rather an object similar to the SearchResults object returned by /repos/search. This pull request fixes this code to return a UserList as per the API description and the SDK.

The issue is that this API has been wrong for at least 10 months - therefore tools may have grown up using the incorrect API. I will also create a pull requests for the opposite solution...

@codecov-io
Copy link

codecov-io commented Sep 2, 2018

Codecov Report

Merging #4846 into master will increase coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4846      +/-   ##
==========================================
+ Coverage   37.31%   37.31%   +<.01%     
==========================================
  Files         305      305              
  Lines       45194    45188       -6     
==========================================
- Hits        16866    16864       -2     
+ Misses      25890    25886       -4     
  Partials     2438     2438
Impacted Files Coverage Δ
routers/api/v1/user/user.go 12.19% <0%> (+0.83%) ⬆️
models/repo_list.go 56.37% <0%> (-1.35%) ⬇️

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 d293a2b...5d92d6c. Read the comment docs.

@bkcsoft bkcsoft added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 2, 2018
@zeripath
Copy link
Contributor Author

zeripath commented Sep 2, 2018

The opposite solutions are provided by go-gitea/go-sdk#119 and #4847. If these are chosen then this request should be cancelled.

@zeripath
Copy link
Contributor Author

zeripath commented Sep 2, 2018

This request will require go-gitea/go-sdk#120 to correct the SDK to match.

@zeripath zeripath force-pushed the Make-Users-Search-Return-A-UserList branch from 20c85ab to 5d92d6c Compare September 7, 2018 09:31
@sapk
Copy link
Member

sapk commented Sep 18, 2018

I think you can close this one since the other solution already have 2 L-GTM. Thanks for the fix.

@zeripath zeripath closed this Sep 18, 2018
@zeripath zeripath deleted the Make-Users-Search-Return-A-UserList branch September 21, 2018 18:15
@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/need 2 This PR needs two approvals by maintainers to be considered for merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants