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.

Sign up for GitHub

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

Jump to bottom

api: Add missing GET teams endpoints #5382

Merged
merged 6 commits into from
Jan 17, 2019
Merged

api: Add missing GET teams endpoints #5382

merged 6 commits into from
Jan 17, 2019

Conversation

HarshitOnGitHub
Copy link
Contributor

This PR adds missing endpoints for teams as mentioned in issue #1821. I have not added endpoints for admin usage in this PR, will open a separate PR for it.
As this is my first PR to this project, I would appreciate a detailed review. Thanks! :)

Partly fixes: #1821.

@codecov-io
Copy link

codecov-io commented Nov 22, 2018 •
edited
Loading

Codecov Report

Merging #5382 into master will decrease coverage by 0.02%.
The diff coverage is 20.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5382      +/-   ##
==========================================
- Coverage   37.76%   37.74%   -0.03%     
==========================================
  Files         325      325              
  Lines       47695    47760      +65     
==========================================
+ Hits        18014    18026      +12     
- Misses      27085    27138      +53     
  Partials     2596     2596
Impacted Files Coverage Δ
routers/api/v1/org/team.go 4.01% <0%> (-0.61%) ⬇️
models/org.go 69.3% <100%> (ø) ⬆️
routers/api/v1/api.go 73.61% <100%> (+0.15%) ⬆️
models/org_team.go 50% <100%> (+0.69%) ⬆️
models/repo_list.go 64.55% <0%> (+1.26%) ⬆️

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 734834a...3ca262f. Read the comment docs.

@bkcsoft bkcsoft added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 22, 2018
lunny
lunny requested changes Nov 23, 2018
View reviewed changes
routers/api/v1/api.go Show resolved Hide resolved
routers/api/v1/org/team.go Show resolved Hide resolved
@techknowlogick techknowlogick added the modifies/api This PR adds API routes or modifies them label Nov 23, 2018
@HarshitOnGitHub
Copy link
Contributor Author

@lunny Just a gentle ping in case you missed github notification for my reply. :)

@lunny
Copy link
Member

lunny commented Dec 2, 2018

@HarshitOnGitHub for compatible with github api v3, the route should be /user/teams, see https://developer.github.com/v3/teams/#list-user-teams, but currently you are in /teams? And the permission is right but I think maybe need an organization field on the return team list.

@techknowlogick techknowlogick added this to the 1.8.0 milestone Dec 27, 2018
@HarshitOnGitHub HarshitOnGitHub mentioned this pull request Dec 29, 2018
Merged
@HarshitOnGitHub
Copy link
Contributor Author

@lunny Thanks for the review! I have updated the PR according to your comments, please take a look. :)

lunny
lunny reviewed Dec 31, 2018
View reviewed changes
routers/api/v1/org/team.go Outdated Show resolved Hide resolved
@HarshitOnGitHub
api: Add an endpoint to list a particular member of team.
15c2564
@HarshitOnGitHub
models: Rename GetUserTeams() to GetUserOrgTeams() in org_team …
fdf1d98 
…model.

`GetUserTeams()` sounds a bit misnomer since it actually returns
the teams that user belongs to in a given organization rather than
all the teams across all the organization that the user has joined.
@HarshitOnGitHub
models: Add GetUserTeams().
c0211a1 
Returns all the teams that a user belongs to.
@HarshitOnGitHub
api: Add an endpoint for GET '/user/teams'.
f530a7f 
A GET request to this endpoint lists all the teams that a user
belongs to.
@HarshitOnGitHub
Copy link
Contributor Author

@lunny This is ready for a final review from you.

@bkcsoft bkcsoft 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 9, 2019
@bkcsoft bkcsoft 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 Jan 17, 2019
@techknowlogick techknowlogick merged commit 5ac6da3 into go-gitea:master Jan 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.
Reviewers

@techknowlogick techknowlogick techknowlogick approved these changes

@lunny lunny lunny approved these changes

Assignees
No one assigned
Labels
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
Projects
None yet
Milestone
1.8.0
Development

Successfully merging this pull request may close these issues.

API : missing GET teams endpoint
5 participants
@HarshitOnGitHub @codecov-io @lunny @techknowlogick @bkcsoft