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

Add API to query collaborators permission for a repository #18761

Merged
merged 13 commits into from
Apr 29, 2022
Merged

Add API to query collaborators permission for a repository #18761

merged 13 commits into from
Apr 29, 2022

Conversation

flozzone
Copy link
Contributor

@flozzone flozzone commented Feb 14, 2022 •
edited
Loading

Targeting #14936, #15332

Adds a collaborator permissions API endpoint according to GitHub API: https://docs.github.com/en/rest/collaborators/collaborators#get-repository-permissions-for-a-user to retrieve a collaborators permissions for a specific repository.

Checks the repository permissions of a collaborator.

GET /repos/{owner}/{repo}/collaborators/{collaborator}/permission

Possible permission values are admin, write, read, owner, none.

{
  "permission": "admin",
  "role_name": "admin",
  "user": {}
}

Where permission and role_name hold the same permission value and user is filled with the user API object. Only admins are allowed to use this API endpoint.

@zeripath zeripath changed the title Add API to query collaborators permission for a repository (#14936) Add API to query collaborators permission for a repository Feb 14, 2022
@zeripath zeripath added modifies/api This PR adds API routes or modifies them type/enhancement An improvement of existing functionality labels Feb 14, 2022
@zeripath zeripath added this to the 1.17.0 milestone Feb 14, 2022
@flozzone flozzone force-pushed the fix-14936 branch 2 times, most recently from d0490e8 to 10c0876 Compare April 27, 2022 06:53
@codecov-commenter
Copy link

codecov-commenter commented Apr 27, 2022 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (main@8eb1cd9). Click here to learn what that means.
The diff coverage is 81.66%.

@@           Coverage Diff           @@
##             main   #18761   +/-   ##
=======================================
  Coverage        ?   47.37%           
=======================================
  Files           ?      952           
  Lines           ?   132516           
  Branches        ?        0           
=======================================
  Hits            ?    62776           
  Misses          ?    62178           
  Partials        ?     7562
Impacted Files Coverage Δ
routers/api/v1/repo/collaborators.go 41.09% <77.08%> (ø)
modules/convert/user.go 79.16% <100.00%> (ø)
routers/api/v1/api.go 75.75% <100.00%> (ø)

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 8eb1cd9...1ba8b23. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 27, 2022
@flozzone
Copy link
Contributor Author

GitHub API endpoint returns the permissions admin, write, read, and none. But Gitea also has owner in AccessMode. Should we just map the owner permission to the admin permission to be compatible with GitHub API or should we return the owner permission if the given user is also a owner of the repository?

Since this is a design question, I would really be happy to get a input from a author.

@lunny
Copy link
Member

lunny commented Apr 27, 2022

GitHub API endpoint returns the permissions admin, write, read, and none. But Gitea also has owner in AccessMode. Should we just map the owner permission to the admin permission to be compatible with GitHub API or should we return the owner permission if the given user is also a owner of the repository?

Since this is a design question, I would really be happy to get a input from a author.

I vote to have an owner permission which should be equal to admin in Github.

@flozzone
Copy link
Contributor Author

The current solution is to return the serialized form of AccessMode, which returns either: admin, write, read, owner, none.

I'll mention this in the PR description.

lunny
lunny previously requested changes Apr 27, 2022
View reviewed changes
Copy link
Member

@lunny lunny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my comment

@flozzone flozzone requested a review from lunny April 27, 2022 12:16
6543
6543 reviewed Apr 27, 2022
View reviewed changes
routers/api/v1/api.go Outdated Show resolved Hide resolved
6543
6543 reviewed Apr 27, 2022
View reviewed changes
models/repo_collaboration.go Outdated Show resolved Hide resolved
@6543

This comment was marked as outdated.

Sign in to view
6543
6543 reviewed Apr 27, 2022
View reviewed changes
models/repo_collaboration.go Outdated Show resolved Hide resolved
@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 27, 2022
@6543
Copy link
Member

6543 commented Apr 27, 2022

make generate-swagger

@flozzone
Copy link
Contributor Author

you should not squash, that will only make reviews harder - it will be squash merged by us

Ok, I was actually not sure about this, I won't squash next time 👍

@6543
Copy link
Member

6543 commented Apr 27, 2022

you can test your tests localy too:

make clean backend test-sqlite\#TestAPIRepoCollaboratorPermission

6543
6543 approved these changes Apr 27, 2022
View reviewed changes
Copy link
Member

@6543 6543 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how could i overlooke that 😅 - thats why tests are nice - works localy

@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 28, 2022
@6543
Copy link
Member

6543 commented Apr 29, 2022

🚀

@6543 6543 merged commit ad6d08d into go-gitea:main Apr 29, 2022
@6543 6543 mentioned this pull request Apr 29, 2022
Closed
6 tasks
Gusted pushed a commit to Gusted/gitea that referenced this pull request Apr 30, 2022
@flozzone @Gusted
Add API to query collaborators permission for a repository (go-gitea#…
7bcee58 
…18761)

Targeting go-gitea#14936, go-gitea#15332

Adds a collaborator permissions API endpoint according to GitHub API: https://docs.github.com/en/rest/collaborators/collaborators#get-repository-permissions-for-a-user to retrieve a collaborators permissions for a specific repository.

### Checks the repository permissions of a collaborator. 

`GET` `/repos/{owner}/{repo}/collaborators/{collaborator}/permission`

Possible `permission` values are `admin`, `write`, `read`, `owner`, `none`.

```json
{
  "permission": "admin",
  "role_name": "admin",
  "user": {}
}
```

Where `permission` and `role_name` hold the same `permission` value and `user` is filled with the user API object. Only admins are allowed to use this API endpoint.
zjjhot added a commit to zjjhot/gitea that referenced this pull request May 1, 2022
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
6f9a52f 
* giteaofficial/main:
  Avoid MoreThanOne Error (go-gitea#19557)
  [skip ci] Updated licenses and gitignores
  Simplify loops to copy (go-gitea#19569)
  Use middleware to open gitRepo (go-gitea#19559)
  Added X-Mailer header to outgoing emails (go-gitea#19562)
  fix go-gitea#19545 (go-gitea#19563)
  [skip ci] Updated translations via Crowdin
  Respect DefaultUserIsRestricted system default when creating new user (go-gitea#19310)
  Mute link in diff header (go-gitea#19556)
  Add API to query collaborators permission for a repository (go-gitea#18761)
  Permalink files In PR diff (go-gitea#19534)
  Fix Pull Request comment filename word breaks (go-gitea#19535)
  Don't error when branch's commit doesn't exist (go-gitea#19547)
@flozzone flozzone deleted the fix-14936 branch June 20, 2022 15:29
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this pull request Aug 24, 2022
@flozzone @AbdulrhmnGhanem
Add API to query collaborators permission for a repository (go-gitea#…
40eb3aa 
…18761)

Targeting go-gitea#14936, go-gitea#15332

Adds a collaborator permissions API endpoint according to GitHub API: https://docs.github.com/en/rest/collaborators/collaborators#get-repository-permissions-for-a-user to retrieve a collaborators permissions for a specific repository.

`GET` `/repos/{owner}/{repo}/collaborators/{collaborator}/permission`

Possible `permission` values are `admin`, `write`, `read`, `owner`, `none`.

```json
{
  "permission": "admin",
  "role_name": "admin",
  "user": {}
}
```

Where `permission` and `role_name` hold the same `permission` value and `user` is filled with the user API object. Only admins are allowed to use this API endpoint.
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@6543 6543 6543 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 type/enhancement An improvement of existing functionality
Projects
None yet
Milestone
1.17.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@flozzone @codecov-commenter @lunny @6543 @zeripath @GiteaBot