Add API branch protection endpoint #9311

davidsvantesson · 2019-12-10T07:48:04Z

Fix #2922.
Replaces #7093.

I have tried to make this work if we get wildcard protected branches as well. @lafriks maybe you want to have a look.

The new API endpoints:

GET /repos/{owner}/{repo}/branch_protections   (Get list)
POST /repos/{owner}/{repo}/branch_protections  (Create new)
GET /repos/{owner}/{repo}/branch_protections/{branch} (Get a specific))
PATCH /repos/{owner}/{repo}/branch_protections/{branch} (Edit)
DELETE /repos/{owner}/{repo}/branch_protections/{branch}

Also added name of effective branch protection of a branch for repo admins to get more information:

GET /repos/{owner}/{repo}/branches/{branch}

6543 · 2019-12-10T11:31:42Z

@davidsvantesson exist there a github a APIv3 reverence?

davidsvantesson · 2019-12-10T18:37:02Z

@6543 It is not compatible with the Github API.

IMHO Github has made a mistake and made API endpoint for branch and branch protection the same. Therefore they don't support wildcard protected branches via API, you can only set branch protection of actual branches. If we intend to implement wildcard protection (#2529), I think we should take the possibility to consider the API while it is not created yet.
Another idea I had with this design is to allow change the branch that is protected (specifically for wildcard protection) without having to delete and create the branch protection again.

The original PR is more similar to Github API. I could take up and fix the issues in that one instead, but I think this matter should be discussed first.

modules/convert/convert.go

modules/structs/repo_branch.go

models/user.go

routers/api/v1/repo/branch.go

6543

some smal nits

davidsvantesson · 2020-01-15T07:28:44Z

I saw some comment in other issue (from @lafriks I believe) that we should change API endpoints to use teamnames rather than team ids. Then it would make sense to use it for this API also I think.

We could also use branch name (or branch protection name in case of wildcard protection) as the API endpoint, but it might require the requester to encode the name in case of wildcards?
In that case I think we should use these endpoints:

/repos/{owner}/{repo}/branch_protection/
/repos/{owner}/{repo}/branch_protection/{branchprotection}

Then it would be similar to GitHub but with a different endpoint name, so that branch protection does not have to be under a specific real branch.

routers/api/v1/repo/branch.go

routers/api/v1/api.go

6543 · 2020-01-15T09:37:12Z

I would take https://github.com/go-gitea/gitea/blob/master/CONTRIBUTING.md#api-v1 into account

and use Patch for the edit action ...
also all status codes are now formated by enum ...

# Conflicts: # modules/convert/convert.go # routers/api/v1/repo/branch.go # routers/api/v1/swagger/options.go # templates/swagger/v1_json.tmpl

codecov-io · 2020-01-18T20:08:13Z

Codecov Report

Merging #9311 into master will decrease coverage by 0.08%.
The diff coverage is 32.94%.

@@            Coverage Diff             @@
##           master    #9311      +/-   ##
==========================================
- Coverage    43.7%   43.61%   -0.09%     
==========================================
  Files         576      583       +7     
  Lines       80251    80801     +550     
==========================================
+ Hits        35070    35239     +169     
- Misses      40851    41191     +340     
- Partials     4330     4371      +41

Impacted Files	Coverage Δ
routers/org/home.go	`65.68% <ø> (-0.34%)`	⬇️
modules/structs/hook.go	`6.66% <ø> (ø)`	⬆️
models/migrations/migrations.go	`4.16% <ø> (ø)`	⬆️
routers/user/profile.go	`50.46% <ø> (-0.24%)`	⬇️
routers/init.go	`63.04% <0%> (-2.13%)`	⬇️
modules/webhook/webhook.go	`43.6% <0%> (-1.71%)`	⬇️
modules/webhook/feishu.go	`0% <0%> (ø)`
modules/graceful/manager_unix.go	`40.83% <0%> (ø)`	⬆️
models/webhook.go	`68.7% <0%> (-0.48%)`	⬇️
modules/queue/unique_queue_redis.go	`3.17% <0%> (ø)`	⬆️
... and 36 more

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 122fb8c...35300c3. Read the comment docs.

lafriks · 2020-02-02T00:55:50Z

I wonder how github api deals with wildcards as github does support them

lunny · 2020-02-02T01:59:57Z

Maybe we should support wildcards branch protection at first.

davidsvantesson · 2020-02-02T14:02:01Z

@lafriks Github doesn't support wildcard branches through their API, you can only set them via UI. We should avoid that.

https://github.community/t5/GitHub-API-Development-and/REST-API-v3-wildcard-branch-protection/td-p/14547

6543 · 2020-02-02T16:28:07Z

routers/api/v1/repo/branch.go

+ // schema:
+ // "$ref": "#/definitions/CreateBranchProtectionOption"
+ // responses:
+ // "200":


Suggested change

// "200":

// "201":

6543 · 2020-02-02T16:28:37Z

routers/api/v1/repo/branch.go

+ return
+ }
+
+ ctx.JSON(http.StatusOK, convert.ToBranchProtection(bp))


Suggested change

ctx.JSON(http.StatusOK, convert.ToBranchProtection(bp))

ctx.JSON(http.StatusCreated, convert.ToBranchProtection(bp))

6543

correct http status responce

6543 · 2020-02-10T14:29:17Z

ping

zeripath · 2020-02-12T23:19:21Z

Make lg-tm work

davidsvantesson added 2 commits December 10, 2019 08:35

add API branch protection endpoint

5e0302a

lint

f0e082d

lunny added modifies/api This PR adds API routes or modifies them type/feature Completely new functionality. Can only be merged if feature freeze is not active. labels Dec 10, 2019

lunny added this to the 1.12.0 milestone Dec 10, 2019

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 10, 2019

guillep2k reviewed Dec 11, 2019

View reviewed changes

6543 reviewed Dec 17, 2019

View reviewed changes

models/user.go Outdated Show resolved Hide resolved

6543 reviewed Dec 17, 2019

View reviewed changes