From 5e0302a4c752a3d8935526203b7427ca03284474 Mon Sep 17 00:00:00 2001 From: David Svantesson Date: Sun, 17 Nov 2019 08:31:24 +0100 Subject: [PATCH 01/13] add API branch protection endpoint --- models/user.go | 10 + modules/convert/convert.go | 82 ++++- modules/structs/repo_branch.go | 83 ++++- routers/api/v1/api.go | 9 + routers/api/v1/repo/branch.go | 378 +++++++++++++++++++++- routers/api/v1/swagger/options.go | 6 + routers/api/v1/swagger/repo.go | 14 + templates/swagger/v1_json.tmpl | 516 +++++++++++++++++++++++++++++- 8 files changed, 1070 insertions(+), 28 deletions(-) diff --git a/models/user.go b/models/user.go index 2cef2e5deccc..9ba8b73dac98 100644 --- a/models/user.go +++ b/models/user.go @@ -1322,6 +1322,16 @@ func GetMaileableUsersByIDs(ids []int64) ([]*User, error) { Find(&ous) } +// GetUsernamesByIDs returns usernames for all resolved users from a list of Ids. +func GetUsernamesByIDs(ids []int64) ([]string, error) { + ous, err := GetUsersByIDs(ids) + unames := make([]string, 0, len(ous)) + for _, u := range ous { + unames = append(unames, u.Name) + } + return unames, err +} + // GetUsersByIDs returns all resolved users from a list of Ids. func GetUsersByIDs(ids []int64) ([]*User, error) { ous := make([]*User, 0, len(ids)) diff --git a/modules/convert/convert.go b/modules/convert/convert.go index 4479653174ca..8f117a8b9861 100644 --- a/modules/convert/convert.go +++ b/modules/convert/convert.go @@ -30,28 +30,76 @@ func ToEmail(email *models.EmailAddress) *api.Email { } // ToBranch convert a git.Commit and git.Branch to an api.Branch -func ToBranch(repo *models.Repository, b *git.Branch, c *git.Commit, bp *models.ProtectedBranch, user *models.User) *api.Branch { +func ToBranch(repo *models.Repository, b *git.Branch, c *git.Commit, bp *models.ProtectedBranch, user *models.User, isRepoAdmin bool) *api.Branch { if bp == nil { return &api.Branch{ - Name: b.Name, - Commit: ToCommit(repo, c), - Protected: false, - RequiredApprovals: 0, - EnableStatusCheck: false, - StatusCheckContexts: []string{}, - UserCanPush: true, - UserCanMerge: true, + Name: b.Name, + Commit: ToCommit(repo, c), + Protected: false, + RequiredApprovals: 0, + EnableStatusCheck: false, + StatusCheckContexts: []string{}, + UserCanPush: true, + UserCanMerge: true, + EffectiveBranchProtectionName: "", + EffectiveBRanchProtectionID: 0, } } + branchProtectionName := "" + var branchProtectionID int64 + if isRepoAdmin { + branchProtectionName = bp.BranchName + branchProtectionID = bp.ID + } + return &api.Branch{ - Name: b.Name, - Commit: ToCommit(repo, c), - Protected: true, - RequiredApprovals: bp.RequiredApprovals, - EnableStatusCheck: bp.EnableStatusCheck, - StatusCheckContexts: bp.StatusCheckContexts, - UserCanPush: bp.CanUserPush(user.ID), - UserCanMerge: bp.CanUserMerge(user.ID), + Name: b.Name, + Commit: ToCommit(repo, c), + Protected: true, + RequiredApprovals: bp.RequiredApprovals, + EnableStatusCheck: bp.EnableStatusCheck, + StatusCheckContexts: bp.StatusCheckContexts, + UserCanPush: bp.CanUserPush(user.ID), + UserCanMerge: bp.CanUserMerge(user.ID), + EffectiveBranchProtectionName: branchProtectionName, + EffectiveBRanchProtectionID: branchProtectionID, + } +} + +// ToBranchProtection convert a ProtectedBranch to api.BranchProtection +func ToBranchProtection(bp *models.ProtectedBranch) *api.BranchProtection { + pushWhitelistUsernames, err := models.GetUsernamesByIDs(bp.WhitelistUserIDs) + if err != nil { + log.Error("GetUsernamesByIDs (WhitelistUserIDs): %v", err) + } + mergeWhitelistUsernames, err := models.GetUsernamesByIDs(bp.MergeWhitelistUserIDs) + if err != nil { + log.Error("GetUsernamesByIDs (MergeWhitelistUserIDs): %v", err) + } + approvalsWhitelistUsernames, err := models.GetUsernamesByIDs(bp.ApprovalsWhitelistUserIDs) + if err != nil { + log.Error("GetUsernamesByIDs (ApprovalsWhitelistUserIDs): %v", err) + } + + return &api.BranchProtection{ + ID: bp.ID, + BranchName: bp.BranchName, + EnablePush: bp.CanPush, + EnablePushWhitelist: bp.EnableWhitelist, + PushWhitelistUsernames: pushWhitelistUsernames, + PushWhitelistTeamIDs: bp.WhitelistTeamIDs, + PushWhitelistDeployKeys: bp.WhitelistDeployKeys, + EnableMergeWhitelist: bp.EnableMergeWhitelist, + MergeWhitelistUsernames: mergeWhitelistUsernames, + MergeWhitelistTeamIDs: bp.MergeWhitelistTeamIDs, + EnableStatusCheck: bp.EnableStatusCheck, + StatusCheckContexts: bp.StatusCheckContexts, + RequiredApprovals: bp.RequiredApprovals, + EnableApprovalsWhitelist: bp.EnableApprovalsWhitelist, + ApprovalsWhitelistUsernames: approvalsWhitelistUsernames, + ApprovalsWhitelistTeamIDs: bp.ApprovalsWhitelistTeamIDs, + Created: bp.CreatedUnix.AsTime(), + Updated: bp.UpdatedUnix.AsTime(), } } diff --git a/modules/structs/repo_branch.go b/modules/structs/repo_branch.go index 42bb7638938b..0c82936cc338 100644 --- a/modules/structs/repo_branch.go +++ b/modules/structs/repo_branch.go @@ -4,14 +4,81 @@ package structs +import ( + "time" +) + // Branch represents a repository branch type Branch struct { - Name string `json:"name"` - Commit *PayloadCommit `json:"commit"` - Protected bool `json:"protected"` - RequiredApprovals int64 `json:"required_approvals"` - EnableStatusCheck bool `json:"enable_status_check"` - StatusCheckContexts []string `json:"status_check_contexts"` - UserCanPush bool `json:"user_can_push"` - UserCanMerge bool `json:"user_can_merge"` + Name string `json:"name"` + Commit *PayloadCommit `json:"commit"` + Protected bool `json:"protected"` + RequiredApprovals int64 `json:"required_approvals"` + EnableStatusCheck bool `json:"enable_status_check"` + StatusCheckContexts []string `json:"status_check_contexts"` + UserCanPush bool `json:"user_can_push"` + UserCanMerge bool `json:"user_can_merge"` + EffectiveBranchProtectionName string `json:"effective_branch_protection_name"` + EffectiveBRanchProtectionID int64 `json:"effective_branch_protection_id"` +} + +// BranchProtection represents a branch protection for a repository +type BranchProtection struct { + ID int64 `json:"id"` + BranchName string `json:"branch_name"` + EnablePush bool `json:"enable_push"` + EnablePushWhitelist bool `json:"enable_push_whitelist"` + PushWhitelistUsernames []string `json:"push_whitelist_usernames"` + PushWhitelistTeamIDs []int64 `json:"push_whitelist_team_ids"` + PushWhitelistDeployKeys bool `json:"push_whitelist_deploy_keys"` + EnableMergeWhitelist bool `json:"enable_merge_whitelist"` + MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"` + MergeWhitelistTeamIDs []int64 `json:"merge_whitelist_ids"` + EnableStatusCheck bool `json:"enable_status_check"` + StatusCheckContexts []string `json:"status_check_contexts"` + RequiredApprovals int64 `json:"required_approvals"` + EnableApprovalsWhitelist bool `json:"enable_approvals_whitelist"` + ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"` + ApprovalsWhitelistTeamIDs []int64 `json:"approvals_whitelist_team_ids"` + // swagger:strfmt date-time + Created time.Time `json:"created_at"` + // swagger:strfmt date-time + Updated time.Time `json:"updated_at"` +} + +// CreateBranchProtectionOption options for creating a branch protection +type CreateBranchProtectionOption struct { + BranchName string `json:"branch_name"` + EnablePush bool `json:"enable_push"` + EnablePushWhitelist bool `json:"enable_push_whitelist"` + PushWhitelistUsernames []string `json:"push_whitelist_usernames"` + PushWhitelistTeamIDs []int64 `json:"push_whitelist_team_ids"` + PushWhitelistDeployKeys bool `json:"push_whitelist_deploy_keys"` + EnableMergeWhitelist bool `json:"enable_merge_whitelist"` + MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"` + MergeWhitelistTeamIDs []int64 `json:"merge_whitelist_team_ids"` + EnableStatusCheck bool `json:"enable_status_check"` + StatusCheckContexts []string `json:"status_check_contexts"` + RequiredApprovals int64 `json:"required_approvals"` + EnableApprovalsWhitelist bool `json:"enable_approvals_whitelist"` + ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"` + ApprovalsWhitelistTeamIDs []int64 `json:"approvals_whitelist_team_ids"` +} + +// EditBranchProtectionOption options for editing a branch protection +type EditBranchProtectionOption struct { + EnablePush *bool `json:"enable_push"` + EnablePushWhitelist *bool `json:"enable_push_whitelist"` + PushWhitelistUsernames []string `json:"push_whitelist_usernames"` + PushWhitelistTeamIDs []int64 `json:"push_whitelist_team_ids"` + PushWhitelistDeployKeys *bool `json:"push_whitelist_deploy_keys"` + EnableMergeWhitelist *bool `json:"enable_merge_whitelist"` + MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"` + MergeWhitelistTeamIDs []int64 `json:"merge_whitelist_team_ids"` + EnableStatusCheck *bool `json:"enable_status_check"` + StatusCheckContexts []string `json:"status_check_contexts"` + RequiredApprovals *int64 `json:"required_approvals"` + EnableApprovalsWhitelist *bool `json:"enable_approvals_whitelist"` + ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"` + ApprovalsWhitelistTeamIDs []int64 `json:"approvals_whitelist_team_ids"` } diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 47c9c95c7fe0..edccad2e9786 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -639,6 +639,15 @@ func RegisterRoutes(m *macaron.Macaron) { m.Get("", repo.ListBranches) m.Get("/*", context.RepoRefByType(context.RepoRefBranch), repo.GetBranch) }, reqRepoReader(models.UnitTypeCode)) + m.Group("/branch_protections", func() { + m.Get("", repo.ListBranchProtections) + m.Post("", bind(api.CreateBranchProtectionOption{}), repo.CreateBranchProtection) + m.Group("/:id", func() { + m.Get("", repo.GetBranchProtection) + m.Put("", bind(api.EditBranchProtectionOption{}), repo.EditBranchProtection) + m.Delete("", repo.DeleteBranchProtection) + }) + }, reqToken(), reqAdmin()) m.Group("/tags", func() { m.Get("", repo.ListTags) }, reqRepoReader(models.UnitTypeCode), context.ReferencesGitRepo(true)) diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 9745903a9598..d3dbcfd1c6c3 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -6,6 +6,7 @@ package repo import ( + "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/convert" "code.gitea.io/gitea/modules/git" @@ -67,7 +68,7 @@ func GetBranch(ctx *context.APIContext) { return } - ctx.JSON(200, convert.ToBranch(ctx.Repo.Repository, branch, c, branchProtection, ctx.User)) + ctx.JSON(200, convert.ToBranch(ctx.Repo.Repository, branch, c, branchProtection, ctx.User, ctx.Repo.IsAdmin())) } // ListBranches list all the branches of a repository @@ -109,8 +110,381 @@ func ListBranches(ctx *context.APIContext) { ctx.Error(500, "GetBranchProtection", err) return } - apiBranches[i] = convert.ToBranch(ctx.Repo.Repository, branches[i], c, branchProtection, ctx.User) + apiBranches[i] = convert.ToBranch(ctx.Repo.Repository, branches[i], c, branchProtection, ctx.User, ctx.Repo.IsAdmin()) } ctx.JSON(200, &apiBranches) } + +// GetBranchProtection gets a branch protection +func GetBranchProtection(ctx *context.APIContext) { + // swagger:operation GET /repos/{owner}/{repo}/branch_protections/{id} repository repoGetBranchProtection + // --- + // summary: Get a specific branch protection for the repository + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: id + // in: path + // description: ID of the branch protection + // type: integer + // format: int64 + // required: true + // responses: + // "200": + // "$ref": "#/responses/BranchProtection" + + repo := ctx.Repo.Repository + bpID := ctx.ParamsInt64(":id") + bp, err := models.GetProtectedBranchByID(bpID) + if err != nil { + ctx.Error(500, "GetProtectedBranchByID", err) + return + } + if bp == nil || bp.RepoID != repo.ID { + ctx.NotFound() + return + } + + ctx.JSON(200, convert.ToBranchProtection(bp)) +} + +// ListBranchProtections list branch protections for a repo +func ListBranchProtections(ctx *context.APIContext) { + // swagger:operation GET /repos/{owner}/{repo}/branch_protections repository repoListBranchProtection + // --- + // summary: List branch protections for a repository + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // responses: + // "200": + // "$ref": "#/responses/BranchProtectionList" + + repo := ctx.Repo.Repository + bps, err := repo.GetProtectedBranches() + if err != nil { + ctx.Error(500, "GetProtectedBranches", err) + return + } + apiBps := make([]*api.BranchProtection, len(bps)) + for i := range bps { + apiBps[i] = convert.ToBranchProtection(bps[i]) + } + + ctx.JSON(200, apiBps) +} + +// CreateBranchProtection creates a branch protection for a repo +func CreateBranchProtection(ctx *context.APIContext, form api.CreateBranchProtectionOption) { + // swagger:operation POST /repos/{owner}/{repo}/branch_protections repository repoCreateBranchProtection + // --- + // summary: Create a branch protections for a repository + // consumes: + // - application/json + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: body + // in: body + // schema: + // "$ref": "#/definitions/CreateBranchProtectionOption" + // responses: + // "200": + // "$ref": "#/responses/BranchProtection" + + // Currently protection must match an actual branch + if !git.IsBranchExist(ctx.Repo.Repository.RepoPath(), form.BranchName) { + ctx.NotFound() + return + } + + protectBranch, err := models.GetProtectedBranchBy(ctx.Repo.Repository.ID, form.BranchName) + if err != nil { + ctx.ServerError("GetProtectBranchOfRepoByName", err) + return + } else if protectBranch != nil { + ctx.Error(403, "Branch protection already exist", err) + return + } + + var requiredApprovals int64 + if form.RequiredApprovals > 0 { + requiredApprovals = form.RequiredApprovals + } + + whitelistUsers, err := models.GetUserIDsByNames(form.PushWhitelistUsernames, false) + if err != nil { + ctx.ServerError("GetUserIDsByNames", err) + } + mergeWhitelistUsers, err := models.GetUserIDsByNames(form.MergeWhitelistUsernames, false) + if err != nil { + ctx.ServerError("GetUserIDsByNames", err) + } + approvalsWhitelistUsers, err := models.GetUserIDsByNames(form.ApprovalsWhitelistUsernames, false) + if err != nil { + ctx.ServerError("GetUserIDsByNames", err) + } + + protectBranch = &models.ProtectedBranch{ + ID: 0, + RepoID: ctx.Repo.Repository.ID, + BranchName: form.BranchName, + CanPush: form.EnablePush, + EnableWhitelist: form.EnablePush && form.EnablePushWhitelist, + EnableMergeWhitelist: form.EnableMergeWhitelist, + WhitelistDeployKeys: form.EnablePush && form.EnablePushWhitelist && form.PushWhitelistDeployKeys, + EnableStatusCheck: form.EnableStatusCheck, + StatusCheckContexts: form.StatusCheckContexts, + EnableApprovalsWhitelist: form.EnableApprovalsWhitelist, + RequiredApprovals: requiredApprovals, + } + + err = models.UpdateProtectBranch(ctx.Repo.Repository, protectBranch, models.WhitelistOptions{ + UserIDs: whitelistUsers, + TeamIDs: form.PushWhitelistTeamIDs, + MergeUserIDs: mergeWhitelistUsers, + MergeTeamIDs: form.MergeWhitelistTeamIDs, + ApprovalsUserIDs: approvalsWhitelistUsers, + ApprovalsTeamIDs: form.ApprovalsWhitelistTeamIDs, + }) + if err != nil { + ctx.ServerError("UpdateProtectBranch", err) + return + } + + // Reload from db to get all whitelists + bp, err := models.GetProtectedBranchBy(ctx.Repo.Repository.ID, form.BranchName) + if err != nil { + ctx.Error(500, "GetProtectedBranchByID", err) + return + } + if bp == nil || bp.RepoID != ctx.Repo.Repository.ID { + ctx.Error(500, "New branch protection not found", err) + return + } + + ctx.JSON(200, convert.ToBranchProtection(bp)) + +} + +// EditBranchProtection edits a branch protection for a repo +func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtectionOption) { + // swagger:operation PUT /repos/{owner}/{repo}/branch_protections/{id} repository repoEditBranchProtection + // --- + // summary: Edit a branch protections for a repository. Only fields that are set will be changed + // consumes: + // - application/json + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: id + // in: path + // description: ID of the branch protection + // type: integer + // format: int64 + // required: true + // - name: body + // in: body + // schema: + // "$ref": "#/definitions/EditBranchProtectionOption" + // responses: + // "204": + // "$ref": "#/responses/BranchProtection" + + protectBranch, err := models.GetProtectedBranchByID(ctx.ParamsInt64(":id")) + if err != nil || protectBranch == nil { + ctx.ServerError("GetProtectBranchOfRepoByName", err) + return + } else if protectBranch.RepoID != ctx.Repo.Repository.ID { + ctx.Error(403, "Branch protection for repo does not exist", err) + return + } + + if form.EnablePush != nil { + if *form.EnablePush == false { + protectBranch.CanPush = false + protectBranch.EnableWhitelist = false + protectBranch.WhitelistDeployKeys = false + } else { + protectBranch.CanPush = true + if form.EnablePushWhitelist != nil { + if *form.EnablePushWhitelist == false { + protectBranch.EnableWhitelist = false + protectBranch.WhitelistDeployKeys = false + } else { + protectBranch.EnableWhitelist = true + if form.PushWhitelistDeployKeys != nil { + protectBranch.WhitelistDeployKeys = *form.PushWhitelistDeployKeys + } + } + } + } + } + + if form.EnableMergeWhitelist != nil { + protectBranch.EnableMergeWhitelist = *form.EnableMergeWhitelist + } + + if form.EnableStatusCheck != nil { + protectBranch.StatusCheckContexts = form.StatusCheckContexts + } + + if form.RequiredApprovals != nil && *form.RequiredApprovals >= 0 { + protectBranch.RequiredApprovals = *form.RequiredApprovals + } + + if form.EnableApprovalsWhitelist != nil { + protectBranch.EnableApprovalsWhitelist = *form.EnableApprovalsWhitelist + } + + var whitelistUsers []int64 + if form.PushWhitelistUsernames != nil { + whitelistUsers, err = models.GetUserIDsByNames(form.PushWhitelistUsernames, false) + if err != nil { + ctx.ServerError("GetUserIDsByNames", err) + } + } else { + whitelistUsers = protectBranch.WhitelistUserIDs + } + var whitelistTeams []int64 + if form.PushWhitelistTeamIDs != nil { + whitelistTeams = form.PushWhitelistTeamIDs + } else { + whitelistTeams = protectBranch.WhitelistTeamIDs + } + + var mergeWhitelistUsers []int64 + if form.MergeWhitelistUsernames != nil { + mergeWhitelistUsers, err = models.GetUserIDsByNames(form.MergeWhitelistUsernames, false) + if err != nil { + ctx.ServerError("GetUserIDsByNames", err) + } + } else { + mergeWhitelistUsers = protectBranch.MergeWhitelistUserIDs + } + var mergeWhitelistTeams []int64 + if form.MergeWhitelistTeamIDs != nil { + mergeWhitelistTeams = form.MergeWhitelistTeamIDs + } else { + mergeWhitelistTeams = protectBranch.MergeWhitelistTeamIDs + } + + var approvalsWhitelistUsers []int64 + if form.ApprovalsWhitelistUsernames != nil { + approvalsWhitelistUsers, err = models.GetUserIDsByNames(form.ApprovalsWhitelistUsernames, false) + if err != nil { + ctx.ServerError("GetUserIDsByNames", err) + } + } else { + approvalsWhitelistUsers = protectBranch.ApprovalsWhitelistUserIDs + } + var approvalsWhitelistTeams []int64 + if form.ApprovalsWhitelistTeamIDs != nil { + approvalsWhitelistTeams = form.ApprovalsWhitelistTeamIDs + } else { + approvalsWhitelistTeams = protectBranch.ApprovalsWhitelistTeamIDs + } + + err = models.UpdateProtectBranch(ctx.Repo.Repository, protectBranch, models.WhitelistOptions{ + UserIDs: whitelistUsers, + TeamIDs: whitelistTeams, + MergeUserIDs: mergeWhitelistUsers, + MergeTeamIDs: mergeWhitelistTeams, + ApprovalsUserIDs: approvalsWhitelistUsers, + ApprovalsTeamIDs: approvalsWhitelistTeams, + }) + if err != nil { + ctx.ServerError("UpdateProtectBranch", err) + return + } + + // Reload from db to ensure get all whitelists + bp, err := models.GetProtectedBranchByID(ctx.ParamsInt64(":id")) + if err != nil { + ctx.Error(500, "GetProtectedBranchByID", err) + return + } + if bp == nil || bp.RepoID != ctx.Repo.Repository.ID { + ctx.Error(500, "New branch protection not found", err) + return + } + + ctx.JSON(200, convert.ToBranchProtection(bp)) +} + +// DeleteBranchProtection deletes a branch protection for a repo +func DeleteBranchProtection(ctx *context.APIContext) { + // swagger:operation DELETE /repos/{owner}/{repo}/branch_protections/{id} repository repoDeleteBranchProtection + // --- + // summary: Delete a specific branch protection for the repository + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: id + // in: path + // description: ID of the branch protection + // type: integer + // format: int64 + // required: true + // responses: + // "204": + // "$ref": "#/responses/empty" + + if err := ctx.Repo.Repository.DeleteProtectedBranch(ctx.ParamsInt64(":id")); err != nil { + ctx.ServerError("DeleteProtectedBranch", err) + return + } + + ctx.Status(204) +} diff --git a/routers/api/v1/swagger/options.go b/routers/api/v1/swagger/options.go index 80e4bf422a93..baca3b63af50 100644 --- a/routers/api/v1/swagger/options.go +++ b/routers/api/v1/swagger/options.go @@ -120,4 +120,10 @@ type swaggerParameterBodies struct { // in:body RepoTopicOptions api.RepoTopicOptions + + // in:body + CreateBranchProtectionOption api.CreateBranchProtectionOption + + // in:body + EditBranchProtectinoOption api.EditBranchProtectionOption } diff --git a/routers/api/v1/swagger/repo.go b/routers/api/v1/swagger/repo.go index 4ac5c6d2d50b..2a657f312236 100644 --- a/routers/api/v1/swagger/repo.go +++ b/routers/api/v1/swagger/repo.go @@ -36,6 +36,20 @@ type swaggerResponseBranchList struct { Body []api.Branch `json:"body"` } +// BranchProtection +// swagger:response BranchProtection +type swaggerResponseBranchProtection struct { + // in:body + Body api.BranchProtection `json:"body"` +} + +// BranchProtectionList +// swagger:response BranchProtectionList +type swaggerResponseBranchProtectionList struct { + // in:body + Body []api.BranchProtection `json:"body"` +} + // TagList // swagger:response TagList type swaggerResponseTagList struct { diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 1d8cd3685876..fb3b2e570eba 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -1451,6 +1451,209 @@ } } }, + "/repos/{owner}/{repo}/branch_protections": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "List branch protections for a repository", + "operationId": "repoListBranchProtection", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "$ref": "#/responses/BranchProtectionList" + } + } + }, + "post": { + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Create a branch protections for a repository", + "operationId": "repoCreateBranchProtection", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "name": "body", + "in": "body", + "schema": { + "$ref": "#/definitions/CreateBranchProtectionOption" + } + } + ], + "responses": { + "200": { + "$ref": "#/responses/BranchProtection" + } + } + } + }, + "/repos/{owner}/{repo}/branch_protections/{id}": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Get a specific branch protection for the repository", + "operationId": "repoGetBranchProtection", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "ID of the branch protection", + "name": "id", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "$ref": "#/responses/BranchProtection" + } + } + }, + "put": { + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Edit a branch protections for a repository. Only fields that are set will be changed", + "operationId": "repoEditBranchProtection", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "ID of the branch protection", + "name": "id", + "in": "path", + "required": true + }, + { + "name": "body", + "in": "body", + "schema": { + "$ref": "#/definitions/EditBranchProtectionOption" + } + } + ], + "responses": { + "204": { + "$ref": "#/responses/BranchProtection" + } + } + }, + "delete": { + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Delete a specific branch protection for the repository", + "operationId": "repoDeleteBranchProtection", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "ID of the branch protection", + "name": "id", + "in": "path", + "required": true + } + ], + "responses": { + "204": { + "$ref": "#/responses/empty" + } + } + } + }, "/repos/{owner}/{repo}/branches": { "get": { "produces": [ @@ -7583,6 +7786,15 @@ "commit": { "$ref": "#/definitions/PayloadCommit" }, + "effective_branch_protection_id": { + "type": "integer", + "format": "int64", + "x-go-name": "EffectiveBRanchProtectionID" + }, + "effective_branch_protection_name": { + "type": "string", + "x-go-name": "EffectiveBranchProtectionName" + }, "enable_status_check": { "type": "boolean", "x-go-name": "EnableStatusCheck" @@ -7618,6 +7830,113 @@ }, "x-go-package": "code.gitea.io/gitea/modules/structs" }, + "BranchProtection": { + "description": "BranchProtection represents a branch protection for a repository", + "type": "object", + "properties": { + "approvals_whitelist_team_ids": { + "type": "array", + "items": { + "type": "integer", + "format": "int64" + }, + "x-go-name": "ApprovalsWhitelistTeamIDs" + }, + "approvals_whitelist_username": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "ApprovalsWhitelistUsernames" + }, + "branch_name": { + "type": "string", + "x-go-name": "BranchName" + }, + "created_at": { + "type": "string", + "format": "date-time", + "x-go-name": "Created" + }, + "enable_approvals_whitelist": { + "type": "boolean", + "x-go-name": "EnableApprovalsWhitelist" + }, + "enable_merge_whitelist": { + "type": "boolean", + "x-go-name": "EnableMergeWhitelist" + }, + "enable_push": { + "type": "boolean", + "x-go-name": "EnablePush" + }, + "enable_push_whitelist": { + "type": "boolean", + "x-go-name": "EnablePushWhitelist" + }, + "enable_status_check": { + "type": "boolean", + "x-go-name": "EnableStatusCheck" + }, + "id": { + "type": "integer", + "format": "int64", + "x-go-name": "ID" + }, + "merge_whitelist_ids": { + "type": "array", + "items": { + "type": "integer", + "format": "int64" + }, + "x-go-name": "MergeWhitelistTeamIDs" + }, + "merge_whitelist_usernames": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "MergeWhitelistUsernames" + }, + "push_whitelist_deploy_keys": { + "type": "boolean", + "x-go-name": "PushWhitelistDeployKeys" + }, + "push_whitelist_team_ids": { + "type": "array", + "items": { + "type": "integer", + "format": "int64" + }, + "x-go-name": "PushWhitelistTeamIDs" + }, + "push_whitelist_usernames": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "PushWhitelistUsernames" + }, + "required_approvals": { + "type": "integer", + "format": "int64", + "x-go-name": "RequiredApprovals" + }, + "status_check_contexts": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "StatusCheckContexts" + }, + "updated_at": { + "type": "string", + "format": "date-time", + "x-go-name": "Updated" + } + }, + "x-go-package": "code.gitea.io/gitea/modules/structs" + }, "Comment": { "description": "Comment represents a comment on a commit or issue", "type": "object", @@ -7806,6 +8125,98 @@ }, "x-go-package": "code.gitea.io/gitea/modules/structs" }, + "CreateBranchProtectionOption": { + "description": "CreateBranchProtectionOption options for creating a branch protection", + "type": "object", + "properties": { + "approvals_whitelist_team_ids": { + "type": "array", + "items": { + "type": "integer", + "format": "int64" + }, + "x-go-name": "ApprovalsWhitelistTeamIDs" + }, + "approvals_whitelist_username": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "ApprovalsWhitelistUsernames" + }, + "branch_name": { + "type": "string", + "x-go-name": "BranchName" + }, + "enable_approvals_whitelist": { + "type": "boolean", + "x-go-name": "EnableApprovalsWhitelist" + }, + "enable_merge_whitelist": { + "type": "boolean", + "x-go-name": "EnableMergeWhitelist" + }, + "enable_push": { + "type": "boolean", + "x-go-name": "EnablePush" + }, + "enable_push_whitelist": { + "type": "boolean", + "x-go-name": "EnablePushWhitelist" + }, + "enable_status_check": { + "type": "boolean", + "x-go-name": "EnableStatusCheck" + }, + "merge_whitelist_team_ids": { + "type": "array", + "items": { + "type": "integer", + "format": "int64" + }, + "x-go-name": "MergeWhitelistTeamIDs" + }, + "merge_whitelist_usernames": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "MergeWhitelistUsernames" + }, + "push_whitelist_deploy_keys": { + "type": "boolean", + "x-go-name": "PushWhitelistDeployKeys" + }, + "push_whitelist_team_ids": { + "type": "array", + "items": { + "type": "integer", + "format": "int64" + }, + "x-go-name": "PushWhitelistTeamIDs" + }, + "push_whitelist_usernames": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "PushWhitelistUsernames" + }, + "required_approvals": { + "type": "integer", + "format": "int64", + "x-go-name": "RequiredApprovals" + }, + "status_check_contexts": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "StatusCheckContexts" + } + }, + "x-go-package": "code.gitea.io/gitea/modules/structs" + }, "CreateEmailOption": { "description": "CreateEmailOption options when creating email addresses", "type": "object", @@ -8476,6 +8887,94 @@ }, "x-go-package": "code.gitea.io/gitea/modules/structs" }, + "EditBranchProtectionOption": { + "description": "EditBranchProtectionOption options for editing a branch protection", + "type": "object", + "properties": { + "approvals_whitelist_team_ids": { + "type": "array", + "items": { + "type": "integer", + "format": "int64" + }, + "x-go-name": "ApprovalsWhitelistTeamIDs" + }, + "approvals_whitelist_username": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "ApprovalsWhitelistUsernames" + }, + "enable_approvals_whitelist": { + "type": "boolean", + "x-go-name": "EnableApprovalsWhitelist" + }, + "enable_merge_whitelist": { + "type": "boolean", + "x-go-name": "EnableMergeWhitelist" + }, + "enable_push": { + "type": "boolean", + "x-go-name": "EnablePush" + }, + "enable_push_whitelist": { + "type": "boolean", + "x-go-name": "EnablePushWhitelist" + }, + "enable_status_check": { + "type": "boolean", + "x-go-name": "EnableStatusCheck" + }, + "merge_whitelist_team_ids": { + "type": "array", + "items": { + "type": "integer", + "format": "int64" + }, + "x-go-name": "MergeWhitelistTeamIDs" + }, + "merge_whitelist_usernames": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "MergeWhitelistUsernames" + }, + "push_whitelist_deploy_keys": { + "type": "boolean", + "x-go-name": "PushWhitelistDeployKeys" + }, + "push_whitelist_team_ids": { + "type": "array", + "items": { + "type": "integer", + "format": "int64" + }, + "x-go-name": "PushWhitelistTeamIDs" + }, + "push_whitelist_usernames": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "PushWhitelistUsernames" + }, + "required_approvals": { + "type": "integer", + "format": "int64", + "x-go-name": "RequiredApprovals" + }, + "status_check_contexts": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "StatusCheckContexts" + } + }, + "x-go-package": "code.gitea.io/gitea/modules/structs" + }, "EditDeadlineOption": { "description": "EditDeadlineOption options for creating a deadline", "type": "object", @@ -10874,6 +11373,21 @@ } } }, + "BranchProtection": { + "description": "BranchProtection", + "schema": { + "$ref": "#/definitions/BranchProtection" + } + }, + "BranchProtectionList": { + "description": "BranchProtectionList", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/BranchProtection" + } + } + }, "Comment": { "description": "Comment", "schema": { @@ -11339,7 +11853,7 @@ "parameterBodies": { "description": "parameterBodies", "schema": { - "$ref": "#/definitions/RepoTopicOptions" + "$ref": "#/definitions/EditBranchProtectionOption" } }, "redirect": { From f0e082dd2bbd1c056e909ded9bd5596cb1a74154 Mon Sep 17 00:00:00 2001 From: davidsvantesson Date: Tue, 10 Dec 2019 08:51:26 +0100 Subject: [PATCH 02/13] lint --- routers/api/v1/repo/branch.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index d3dbcfd1c6c3..0408283d8406 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -342,14 +342,14 @@ func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtection } if form.EnablePush != nil { - if *form.EnablePush == false { + if !*form.EnablePush { protectBranch.CanPush = false protectBranch.EnableWhitelist = false protectBranch.WhitelistDeployKeys = false } else { protectBranch.CanPush = true if form.EnablePushWhitelist != nil { - if *form.EnablePushWhitelist == false { + if !*form.EnablePushWhitelist { protectBranch.EnableWhitelist = false protectBranch.WhitelistDeployKeys = false } else { From a46dbc2a1db71939dc992f06d657e0d691ccf411 Mon Sep 17 00:00:00 2001 From: David Svantesson Date: Sat, 18 Jan 2020 20:16:55 +0100 Subject: [PATCH 03/13] Change to use team names instead of ids. --- models/org_team.go | 36 ++++++++++ models/user.go | 4 +- modules/convert/convert.go | 34 ++++++---- modules/structs/repo_branch.go | 19 +++--- routers/api/v1/api.go | 4 +- routers/api/v1/repo/branch.go | 110 ++++++++++++++++++++---------- templates/swagger/v1_json.tmpl | 118 +++++++++++++++------------------ 7 files changed, 198 insertions(+), 127 deletions(-) diff --git a/models/org_team.go b/models/org_team.go index 0c0a1e7b7973..88c6e0f3768c 100644 --- a/models/org_team.go +++ b/models/org_team.go @@ -541,6 +541,23 @@ func GetTeam(orgID int64, name string) (*Team, error) { return getTeam(x, orgID, name) } +// GetTeamIDsByNames returns a slice of team ids corresponds to names. +func GetTeamIDsByNames(orgID int64, names []string, ignoreNonExistent bool) ([]int64, error) { + ids := make([]int64, 0, len(names)) + for _, name := range names { + u, err := GetTeam(orgID, name) + if err != nil { + if ignoreNonExistent { + continue + } else { + return nil, err + } + } + ids = append(ids, u.ID) + } + return ids, nil +} + // getOwnerTeam returns team by given team name and organization. func getOwnerTeam(e Engine, orgID int64) (*Team, error) { return getTeam(e, orgID, ownerTeamName) @@ -562,6 +579,25 @@ func GetTeamByID(teamID int64) (*Team, error) { return getTeamByID(x, teamID) } +// GetTeamNamesByID returns team's lower name from a list of team ids. +func GetTeamNamesByID(teamIDs []int64) ([]string, error) { + if len(teamIDs) == 0 { + return []string{}, nil + } + + teams := make([]*Team, 0, len(teamIDs)) + + err := x.In("id", teamIDs). + Asc("name"). + Find(&teams) + + teamnames := make([]string, 0, len(teams)) + for _, t := range teams { + teamnames = append(teamnames, t.LowerName) + } + return teamnames, err +} + // UpdateTeam updates information of team. func UpdateTeam(t *Team, authChanged bool, includeAllChanged bool) (err error) { if len(t.Name) == 0 { diff --git a/models/user.go b/models/user.go index 93c4f49d4715..1247662130a5 100644 --- a/models/user.go +++ b/models/user.go @@ -1349,8 +1349,8 @@ func GetMaileableUsersByIDs(ids []int64) ([]*User, error) { Find(&ous) } -// GetUsernamesByIDs returns usernames for all resolved users from a list of Ids. -func GetUsernamesByIDs(ids []int64) ([]string, error) { +// GetUserNamesByIDs returns usernames for all resolved users from a list of Ids. +func GetUserNamesByIDs(ids []int64) ([]string, error) { ous, err := GetUsersByIDs(ids) unames := make([]string, 0, len(ous)) for _, u := range ous { diff --git a/modules/convert/convert.go b/modules/convert/convert.go index 2a50bd372812..a45fea399b76 100644 --- a/modules/convert/convert.go +++ b/modules/convert/convert.go @@ -42,14 +42,11 @@ func ToBranch(repo *models.Repository, b *git.Branch, c *git.Commit, bp *models. UserCanPush: true, UserCanMerge: true, EffectiveBranchProtectionName: "", - EffectiveBRanchProtectionID: 0, } } branchProtectionName := "" - var branchProtectionID int64 if isRepoAdmin { branchProtectionName = bp.BranchName - branchProtectionID = bp.ID } return &api.Branch{ @@ -62,23 +59,34 @@ func ToBranch(repo *models.Repository, b *git.Branch, c *git.Commit, bp *models. UserCanPush: bp.CanUserPush(user.ID), UserCanMerge: bp.IsUserMergeWhitelisted(user.ID), EffectiveBranchProtectionName: branchProtectionName, - EffectiveBRanchProtectionID: branchProtectionID, } } // ToBranchProtection convert a ProtectedBranch to api.BranchProtection func ToBranchProtection(bp *models.ProtectedBranch) *api.BranchProtection { - pushWhitelistUsernames, err := models.GetUsernamesByIDs(bp.WhitelistUserIDs) + pushWhitelistUsernames, err := models.GetUserNamesByIDs(bp.WhitelistUserIDs) if err != nil { - log.Error("GetUsernamesByIDs (WhitelistUserIDs): %v", err) + log.Error("GetUserNamesByIDs (WhitelistUserIDs): %v", err) } - mergeWhitelistUsernames, err := models.GetUsernamesByIDs(bp.MergeWhitelistUserIDs) + mergeWhitelistUsernames, err := models.GetUserNamesByIDs(bp.MergeWhitelistUserIDs) if err != nil { - log.Error("GetUsernamesByIDs (MergeWhitelistUserIDs): %v", err) + log.Error("GetUserNamesByIDs (MergeWhitelistUserIDs): %v", err) } - approvalsWhitelistUsernames, err := models.GetUsernamesByIDs(bp.ApprovalsWhitelistUserIDs) + approvalsWhitelistUsernames, err := models.GetUserNamesByIDs(bp.ApprovalsWhitelistUserIDs) if err != nil { - log.Error("GetUsernamesByIDs (ApprovalsWhitelistUserIDs): %v", err) + log.Error("GetUserNamesByIDs (ApprovalsWhitelistUserIDs): %v", err) + } + pushWhitelistTeams, err := models.GetTeamNamesByID(bp.WhitelistTeamIDs) + if err != nil { + log.Error("GetTeamNamesByID (WhitelistTeamIDs): %v", err) + } + mergeWhitelistTeams, err := models.GetTeamNamesByID(bp.MergeWhitelistTeamIDs) + if err != nil { + log.Error("GetTeamNamesByID (MergeWhitelistTeamIDs): %v", err) + } + approvalsWhitelistTeams, err := models.GetTeamNamesByID(bp.ApprovalsWhitelistTeamIDs) + if err != nil { + log.Error("GetTeamNamesByID (ApprovalsWhitelistTeamIDs): %v", err) } return &api.BranchProtection{ @@ -87,17 +95,17 @@ func ToBranchProtection(bp *models.ProtectedBranch) *api.BranchProtection { EnablePush: bp.CanPush, EnablePushWhitelist: bp.EnableWhitelist, PushWhitelistUsernames: pushWhitelistUsernames, - PushWhitelistTeamIDs: bp.WhitelistTeamIDs, + PushWhitelistTeams: pushWhitelistTeams, PushWhitelistDeployKeys: bp.WhitelistDeployKeys, EnableMergeWhitelist: bp.EnableMergeWhitelist, MergeWhitelistUsernames: mergeWhitelistUsernames, - MergeWhitelistTeamIDs: bp.MergeWhitelistTeamIDs, + MergeWhitelistTeams: mergeWhitelistTeams, EnableStatusCheck: bp.EnableStatusCheck, StatusCheckContexts: bp.StatusCheckContexts, RequiredApprovals: bp.RequiredApprovals, EnableApprovalsWhitelist: bp.EnableApprovalsWhitelist, ApprovalsWhitelistUsernames: approvalsWhitelistUsernames, - ApprovalsWhitelistTeamIDs: bp.ApprovalsWhitelistTeamIDs, + ApprovalsWhitelistTeams: approvalsWhitelistTeams, Created: bp.CreatedUnix.AsTime(), Updated: bp.UpdatedUnix.AsTime(), } diff --git a/modules/structs/repo_branch.go b/modules/structs/repo_branch.go index 0c82936cc338..3e5c8c2b2a50 100644 --- a/modules/structs/repo_branch.go +++ b/modules/structs/repo_branch.go @@ -19,7 +19,6 @@ type Branch struct { UserCanPush bool `json:"user_can_push"` UserCanMerge bool `json:"user_can_merge"` EffectiveBranchProtectionName string `json:"effective_branch_protection_name"` - EffectiveBRanchProtectionID int64 `json:"effective_branch_protection_id"` } // BranchProtection represents a branch protection for a repository @@ -29,17 +28,17 @@ type BranchProtection struct { EnablePush bool `json:"enable_push"` EnablePushWhitelist bool `json:"enable_push_whitelist"` PushWhitelistUsernames []string `json:"push_whitelist_usernames"` - PushWhitelistTeamIDs []int64 `json:"push_whitelist_team_ids"` + PushWhitelistTeams []string `json:"push_whitelist_teams"` PushWhitelistDeployKeys bool `json:"push_whitelist_deploy_keys"` EnableMergeWhitelist bool `json:"enable_merge_whitelist"` MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"` - MergeWhitelistTeamIDs []int64 `json:"merge_whitelist_ids"` + MergeWhitelistTeams []string `json:"merge_whitelist_teams"` EnableStatusCheck bool `json:"enable_status_check"` StatusCheckContexts []string `json:"status_check_contexts"` RequiredApprovals int64 `json:"required_approvals"` EnableApprovalsWhitelist bool `json:"enable_approvals_whitelist"` ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"` - ApprovalsWhitelistTeamIDs []int64 `json:"approvals_whitelist_team_ids"` + ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"` // swagger:strfmt date-time Created time.Time `json:"created_at"` // swagger:strfmt date-time @@ -52,17 +51,17 @@ type CreateBranchProtectionOption struct { EnablePush bool `json:"enable_push"` EnablePushWhitelist bool `json:"enable_push_whitelist"` PushWhitelistUsernames []string `json:"push_whitelist_usernames"` - PushWhitelistTeamIDs []int64 `json:"push_whitelist_team_ids"` + PushWhitelistTeams []string `json:"push_whitelist_teams"` PushWhitelistDeployKeys bool `json:"push_whitelist_deploy_keys"` EnableMergeWhitelist bool `json:"enable_merge_whitelist"` MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"` - MergeWhitelistTeamIDs []int64 `json:"merge_whitelist_team_ids"` + MergeWhitelistTeams []string `json:"merge_whitelist_teams"` EnableStatusCheck bool `json:"enable_status_check"` StatusCheckContexts []string `json:"status_check_contexts"` RequiredApprovals int64 `json:"required_approvals"` EnableApprovalsWhitelist bool `json:"enable_approvals_whitelist"` ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"` - ApprovalsWhitelistTeamIDs []int64 `json:"approvals_whitelist_team_ids"` + ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"` } // EditBranchProtectionOption options for editing a branch protection @@ -70,15 +69,15 @@ type EditBranchProtectionOption struct { EnablePush *bool `json:"enable_push"` EnablePushWhitelist *bool `json:"enable_push_whitelist"` PushWhitelistUsernames []string `json:"push_whitelist_usernames"` - PushWhitelistTeamIDs []int64 `json:"push_whitelist_team_ids"` + PushWhitelistTeams []string `json:"push_whitelist_teams"` PushWhitelistDeployKeys *bool `json:"push_whitelist_deploy_keys"` EnableMergeWhitelist *bool `json:"enable_merge_whitelist"` MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"` - MergeWhitelistTeamIDs []int64 `json:"merge_whitelist_team_ids"` + MergeWhitelistTeams []string `json:"merge_whitelist_teams"` EnableStatusCheck *bool `json:"enable_status_check"` StatusCheckContexts []string `json:"status_check_contexts"` RequiredApprovals *int64 `json:"required_approvals"` EnableApprovalsWhitelist *bool `json:"enable_approvals_whitelist"` ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"` - ApprovalsWhitelistTeamIDs []int64 `json:"approvals_whitelist_team_ids"` + ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"` } diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index ea2214fcb950..5d716d713cc7 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -658,9 +658,9 @@ func RegisterRoutes(m *macaron.Macaron) { m.Group("/branch_protections", func() { m.Get("", repo.ListBranchProtections) m.Post("", bind(api.CreateBranchProtectionOption{}), repo.CreateBranchProtection) - m.Group("/:id", func() { + m.Group("/:name", func() { m.Get("", repo.GetBranchProtection) - m.Put("", bind(api.EditBranchProtectionOption{}), repo.EditBranchProtection) + m.Patch("", bind(api.EditBranchProtectionOption{}), repo.EditBranchProtection) m.Delete("", repo.DeleteBranchProtection) }) }, reqToken(), reqAdmin()) diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 69f2eaec1a4e..1623fa09c52b 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -123,7 +123,7 @@ func ListBranches(ctx *context.APIContext) { // GetBranchProtection gets a branch protection func GetBranchProtection(ctx *context.APIContext) { - // swagger:operation GET /repos/{owner}/{repo}/branch_protections/{id} repository repoGetBranchProtection + // swagger:operation GET /repos/{owner}/{repo}/branch_protections/{name} repository repoGetBranchProtection // --- // summary: Get a specific branch protection for the repository // produces: @@ -150,10 +150,10 @@ func GetBranchProtection(ctx *context.APIContext) { // "$ref": "#/responses/BranchProtection" repo := ctx.Repo.Repository - bpID := ctx.ParamsInt64(":id") - bp, err := models.GetProtectedBranchByID(bpID) + bpName := ctx.Params(":name") + bp, err := models.GetProtectedBranchBy(repo.ID, bpName) if err != nil { - ctx.Error(500, "GetProtectedBranchByID", err) + ctx.Error(http.StatusInternalServerError, "GetProtectedBranchByID", err) return } if bp == nil || bp.RepoID != repo.ID { @@ -189,7 +189,7 @@ func ListBranchProtections(ctx *context.APIContext) { repo := ctx.Repo.Repository bps, err := repo.GetProtectedBranches() if err != nil { - ctx.Error(500, "GetProtectedBranches", err) + ctx.Error(http.StatusInternalServerError, "GetProtectedBranches", err) return } apiBps := make([]*api.BranchProtection, len(bps)) @@ -228,13 +228,15 @@ func CreateBranchProtection(ctx *context.APIContext, form api.CreateBranchProtec // "200": // "$ref": "#/responses/BranchProtection" + repo := ctx.Repo.Repository + // Currently protection must match an actual branch if !git.IsBranchExist(ctx.Repo.Repository.RepoPath(), form.BranchName) { ctx.NotFound() return } - protectBranch, err := models.GetProtectedBranchBy(ctx.Repo.Repository.ID, form.BranchName) + protectBranch, err := models.GetProtectedBranchBy(repo.ID, form.BranchName) if err != nil { ctx.ServerError("GetProtectBranchOfRepoByName", err) return @@ -260,6 +262,21 @@ func CreateBranchProtection(ctx *context.APIContext, form api.CreateBranchProtec if err != nil { ctx.ServerError("GetUserIDsByNames", err) } + var whitelistTeams, mergeWhitelistTeams, approvalsWhitelistTeams []int64 + if repo.Owner.IsOrganization() { + whitelistTeams, err = models.GetTeamIDsByNames(repo.OwnerID, form.PushWhitelistTeams, false) + if err != nil { + ctx.ServerError("GetTeamIDsByNames", err) + } + mergeWhitelistTeams, err = models.GetTeamIDsByNames(repo.OwnerID, form.MergeWhitelistTeams, false) + if err != nil { + ctx.ServerError("GetTeamIDsByNames", err) + } + approvalsWhitelistTeams, err = models.GetTeamIDsByNames(repo.OwnerID, form.ApprovalsWhitelistTeams, false) + if err != nil { + ctx.ServerError("GetTeamIDsByNames", err) + } + } protectBranch = &models.ProtectedBranch{ ID: 0, @@ -277,11 +294,11 @@ func CreateBranchProtection(ctx *context.APIContext, form api.CreateBranchProtec err = models.UpdateProtectBranch(ctx.Repo.Repository, protectBranch, models.WhitelistOptions{ UserIDs: whitelistUsers, - TeamIDs: form.PushWhitelistTeamIDs, + TeamIDs: whitelistTeams, MergeUserIDs: mergeWhitelistUsers, - MergeTeamIDs: form.MergeWhitelistTeamIDs, + MergeTeamIDs: mergeWhitelistTeams, ApprovalsUserIDs: approvalsWhitelistUsers, - ApprovalsTeamIDs: form.ApprovalsWhitelistTeamIDs, + ApprovalsTeamIDs: approvalsWhitelistTeams, }) if err != nil { ctx.ServerError("UpdateProtectBranch", err) @@ -305,7 +322,7 @@ func CreateBranchProtection(ctx *context.APIContext, form api.CreateBranchProtec // EditBranchProtection edits a branch protection for a repo func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtectionOption) { - // swagger:operation PUT /repos/{owner}/{repo}/branch_protections/{id} repository repoEditBranchProtection + // swagger:operation PATCH /repos/{owner}/{repo}/branch_protections/{name} repository repoEditBranchProtection // --- // summary: Edit a branch protections for a repository. Only fields that are set will be changed // consumes: @@ -337,12 +354,15 @@ func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtection // "204": // "$ref": "#/responses/BranchProtection" - protectBranch, err := models.GetProtectedBranchByID(ctx.ParamsInt64(":id")) - if err != nil || protectBranch == nil { - ctx.ServerError("GetProtectBranchOfRepoByName", err) + repo := ctx.Repo.Repository + bpName := ctx.Params(":name") + protectBranch, err := models.GetProtectedBranchBy(repo.ID, bpName) + if err != nil { + ctx.Error(500, "GetProtectedBranchByID", err) return - } else if protectBranch.RepoID != ctx.Repo.Repository.ID { - ctx.Error(403, "Branch protection for repo does not exist", err) + } + if protectBranch == nil || protectBranch.RepoID != repo.ID { + ctx.NotFound() return } @@ -392,13 +412,6 @@ func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtection } else { whitelistUsers = protectBranch.WhitelistUserIDs } - var whitelistTeams []int64 - if form.PushWhitelistTeamIDs != nil { - whitelistTeams = form.PushWhitelistTeamIDs - } else { - whitelistTeams = protectBranch.WhitelistTeamIDs - } - var mergeWhitelistUsers []int64 if form.MergeWhitelistUsernames != nil { mergeWhitelistUsers, err = models.GetUserIDsByNames(form.MergeWhitelistUsernames, false) @@ -408,13 +421,6 @@ func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtection } else { mergeWhitelistUsers = protectBranch.MergeWhitelistUserIDs } - var mergeWhitelistTeams []int64 - if form.MergeWhitelistTeamIDs != nil { - mergeWhitelistTeams = form.MergeWhitelistTeamIDs - } else { - mergeWhitelistTeams = protectBranch.MergeWhitelistTeamIDs - } - var approvalsWhitelistUsers []int64 if form.ApprovalsWhitelistUsernames != nil { approvalsWhitelistUsers, err = models.GetUserIDsByNames(form.ApprovalsWhitelistUsernames, false) @@ -424,11 +430,33 @@ func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtection } else { approvalsWhitelistUsers = protectBranch.ApprovalsWhitelistUserIDs } - var approvalsWhitelistTeams []int64 - if form.ApprovalsWhitelistTeamIDs != nil { - approvalsWhitelistTeams = form.ApprovalsWhitelistTeamIDs - } else { - approvalsWhitelistTeams = protectBranch.ApprovalsWhitelistTeamIDs + + var whitelistTeams, mergeWhitelistTeams, approvalsWhitelistTeams []int64 + if repo.Owner.IsOrganization() { + if form.PushWhitelistTeams != nil { + whitelistTeams, err = models.GetTeamIDsByNames(repo.OwnerID, form.PushWhitelistTeams, false) + if err != nil { + ctx.ServerError("GetTeamIDsByNames", err) + } + } else { + whitelistTeams = protectBranch.WhitelistTeamIDs + } + if form.MergeWhitelistTeams != nil { + mergeWhitelistTeams, err = models.GetTeamIDsByNames(repo.OwnerID, form.MergeWhitelistTeams, false) + if err != nil { + ctx.ServerError("GetTeamIDsByNames", err) + } + } else { + mergeWhitelistTeams = protectBranch.MergeWhitelistTeamIDs + } + if form.ApprovalsWhitelistTeams != nil { + approvalsWhitelistTeams, err = models.GetTeamIDsByNames(repo.OwnerID, form.ApprovalsWhitelistTeams, false) + if err != nil { + ctx.ServerError("GetTeamIDsByNames", err) + } + } else { + approvalsWhitelistTeams = protectBranch.ApprovalsWhitelistTeamIDs + } } err = models.UpdateProtectBranch(ctx.Repo.Repository, protectBranch, models.WhitelistOptions{ @@ -486,7 +514,19 @@ func DeleteBranchProtection(ctx *context.APIContext) { // "204": // "$ref": "#/responses/empty" - if err := ctx.Repo.Repository.DeleteProtectedBranch(ctx.ParamsInt64(":id")); err != nil { + repo := ctx.Repo.Repository + bpName := ctx.Params(":name") + bp, err := models.GetProtectedBranchBy(repo.ID, bpName) + if err != nil { + ctx.Error(500, "GetProtectedBranchByID", err) + return + } + if bp == nil || bp.RepoID != repo.ID { + ctx.NotFound() + return + } + + if err := ctx.Repo.Repository.DeleteProtectedBranch(bp.ID); err != nil { ctx.ServerError("DeleteProtectedBranch", err) return } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 14edf578f7dd..4aa0a56aa965 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -1780,15 +1780,15 @@ } }, "/repos/{owner}/{repo}/branch_protections/{id}": { - "get": { + "delete": { "produces": [ "application/json" ], "tags": [ "repository" ], - "summary": "Get a specific branch protection for the repository", - "operationId": "repoGetBranchProtection", + "summary": "Delete a specific branch protection for the repository", + "operationId": "repoDeleteBranchProtection", "parameters": [ { "type": "string", @@ -1814,23 +1814,22 @@ } ], "responses": { - "200": { - "$ref": "#/responses/BranchProtection" + "204": { + "$ref": "#/responses/empty" } } - }, - "put": { - "consumes": [ - "application/json" - ], + } + }, + "/repos/{owner}/{repo}/branch_protections/{name}": { + "get": { "produces": [ "application/json" ], "tags": [ "repository" ], - "summary": "Edit a branch protections for a repository. Only fields that are set will be changed", - "operationId": "repoEditBranchProtection", + "summary": "Get a specific branch protection for the repository", + "operationId": "repoGetBranchProtection", "parameters": [ { "type": "string", @@ -1853,30 +1852,26 @@ "name": "id", "in": "path", "required": true - }, - { - "name": "body", - "in": "body", - "schema": { - "$ref": "#/definitions/EditBranchProtectionOption" - } } ], "responses": { - "204": { + "200": { "$ref": "#/responses/BranchProtection" } } }, - "delete": { + "patch": { + "consumes": [ + "application/json" + ], "produces": [ "application/json" ], "tags": [ "repository" ], - "summary": "Delete a specific branch protection for the repository", - "operationId": "repoDeleteBranchProtection", + "summary": "Edit a branch protections for a repository. Only fields that are set will be changed", + "operationId": "repoEditBranchProtection", "parameters": [ { "type": "string", @@ -1899,11 +1894,18 @@ "name": "id", "in": "path", "required": true + }, + { + "name": "body", + "in": "body", + "schema": { + "$ref": "#/definitions/EditBranchProtectionOption" + } } ], "responses": { "204": { - "$ref": "#/responses/empty" + "$ref": "#/responses/BranchProtection" } } } @@ -8944,11 +8946,6 @@ "commit": { "$ref": "#/definitions/PayloadCommit" }, - "effective_branch_protection_id": { - "type": "integer", - "format": "int64", - "x-go-name": "EffectiveBRanchProtectionID" - }, "effective_branch_protection_name": { "type": "string", "x-go-name": "EffectiveBranchProtectionName" @@ -8992,13 +8989,12 @@ "description": "BranchProtection represents a branch protection for a repository", "type": "object", "properties": { - "approvals_whitelist_team_ids": { + "approvals_whitelist_teams": { "type": "array", "items": { - "type": "integer", - "format": "int64" + "type": "string" }, - "x-go-name": "ApprovalsWhitelistTeamIDs" + "x-go-name": "ApprovalsWhitelistTeams" }, "approvals_whitelist_username": { "type": "array", @@ -9041,13 +9037,12 @@ "format": "int64", "x-go-name": "ID" }, - "merge_whitelist_ids": { + "merge_whitelist_teams": { "type": "array", "items": { - "type": "integer", - "format": "int64" + "type": "string" }, - "x-go-name": "MergeWhitelistTeamIDs" + "x-go-name": "MergeWhitelistTeams" }, "merge_whitelist_usernames": { "type": "array", @@ -9060,13 +9055,12 @@ "type": "boolean", "x-go-name": "PushWhitelistDeployKeys" }, - "push_whitelist_team_ids": { + "push_whitelist_teams": { "type": "array", "items": { - "type": "integer", - "format": "int64" + "type": "string" }, - "x-go-name": "PushWhitelistTeamIDs" + "x-go-name": "PushWhitelistTeams" }, "push_whitelist_usernames": { "type": "array", @@ -9304,13 +9298,12 @@ "description": "CreateBranchProtectionOption options for creating a branch protection", "type": "object", "properties": { - "approvals_whitelist_team_ids": { + "approvals_whitelist_teams": { "type": "array", "items": { - "type": "integer", - "format": "int64" + "type": "string" }, - "x-go-name": "ApprovalsWhitelistTeamIDs" + "x-go-name": "ApprovalsWhitelistTeams" }, "approvals_whitelist_username": { "type": "array", @@ -9343,13 +9336,12 @@ "type": "boolean", "x-go-name": "EnableStatusCheck" }, - "merge_whitelist_team_ids": { + "merge_whitelist_teams": { "type": "array", "items": { - "type": "integer", - "format": "int64" + "type": "string" }, - "x-go-name": "MergeWhitelistTeamIDs" + "x-go-name": "MergeWhitelistTeams" }, "merge_whitelist_usernames": { "type": "array", @@ -9362,13 +9354,12 @@ "type": "boolean", "x-go-name": "PushWhitelistDeployKeys" }, - "push_whitelist_team_ids": { + "push_whitelist_teams": { "type": "array", "items": { - "type": "integer", - "format": "int64" + "type": "string" }, - "x-go-name": "PushWhitelistTeamIDs" + "x-go-name": "PushWhitelistTeams" }, "push_whitelist_usernames": { "type": "array", @@ -10079,13 +10070,12 @@ "description": "EditBranchProtectionOption options for editing a branch protection", "type": "object", "properties": { - "approvals_whitelist_team_ids": { + "approvals_whitelist_teams": { "type": "array", "items": { - "type": "integer", - "format": "int64" + "type": "string" }, - "x-go-name": "ApprovalsWhitelistTeamIDs" + "x-go-name": "ApprovalsWhitelistTeams" }, "approvals_whitelist_username": { "type": "array", @@ -10114,13 +10104,12 @@ "type": "boolean", "x-go-name": "EnableStatusCheck" }, - "merge_whitelist_team_ids": { + "merge_whitelist_teams": { "type": "array", "items": { - "type": "integer", - "format": "int64" + "type": "string" }, - "x-go-name": "MergeWhitelistTeamIDs" + "x-go-name": "MergeWhitelistTeams" }, "merge_whitelist_usernames": { "type": "array", @@ -10133,13 +10122,12 @@ "type": "boolean", "x-go-name": "PushWhitelistDeployKeys" }, - "push_whitelist_team_ids": { + "push_whitelist_teams": { "type": "array", "items": { - "type": "integer", - "format": "int64" + "type": "string" }, - "x-go-name": "PushWhitelistTeamIDs" + "x-go-name": "PushWhitelistTeams" }, "push_whitelist_usernames": { "type": "array", From 637d10fa5a077ac527512bf5db14852e9e38f6ac Mon Sep 17 00:00:00 2001 From: David Svantesson Date: Sat, 18 Jan 2020 20:35:38 +0100 Subject: [PATCH 04/13] Status codes. --- routers/api/v1/repo/branch.go | 36 ++++++++++++++++++++++------------ templates/swagger/v1_json.tmpl | 17 +++++++++++++++- 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 1623fa09c52b..9ce90b33ad80 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -148,6 +148,8 @@ func GetBranchProtection(ctx *context.APIContext) { // responses: // "200": // "$ref": "#/responses/BranchProtection" + // "404": + // "$ref": "#/responses/notFound" repo := ctx.Repo.Repository bpName := ctx.Params(":name") @@ -161,7 +163,7 @@ func GetBranchProtection(ctx *context.APIContext) { return } - ctx.JSON(200, convert.ToBranchProtection(bp)) + ctx.JSON(http.StatusOK, convert.ToBranchProtection(bp)) } // ListBranchProtections list branch protections for a repo @@ -197,7 +199,7 @@ func ListBranchProtections(ctx *context.APIContext) { apiBps[i] = convert.ToBranchProtection(bps[i]) } - ctx.JSON(200, apiBps) + ctx.JSON(http.StatusOK, apiBps) } // CreateBranchProtection creates a branch protection for a repo @@ -227,6 +229,10 @@ func CreateBranchProtection(ctx *context.APIContext, form api.CreateBranchProtec // responses: // "200": // "$ref": "#/responses/BranchProtection" + // "403": + // "$ref": "#/responses/forbidden" + // "404": + // "$ref": "#/responses/notFound" repo := ctx.Repo.Repository @@ -241,7 +247,7 @@ func CreateBranchProtection(ctx *context.APIContext, form api.CreateBranchProtec ctx.ServerError("GetProtectBranchOfRepoByName", err) return } else if protectBranch != nil { - ctx.Error(403, "Branch protection already exist", err) + ctx.Error(http.StatusForbidden, "Branch protection already exist", err) return } @@ -308,15 +314,15 @@ func CreateBranchProtection(ctx *context.APIContext, form api.CreateBranchProtec // Reload from db to get all whitelists bp, err := models.GetProtectedBranchBy(ctx.Repo.Repository.ID, form.BranchName) if err != nil { - ctx.Error(500, "GetProtectedBranchByID", err) + ctx.Error(http.StatusInternalServerError, "GetProtectedBranchByID", err) return } if bp == nil || bp.RepoID != ctx.Repo.Repository.ID { - ctx.Error(500, "New branch protection not found", err) + ctx.Error(http.StatusInternalServerError, "New branch protection not found", err) return } - ctx.JSON(200, convert.ToBranchProtection(bp)) + ctx.JSON(http.StatusOK, convert.ToBranchProtection(bp)) } @@ -351,14 +357,16 @@ func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtection // schema: // "$ref": "#/definitions/EditBranchProtectionOption" // responses: - // "204": + // "200": // "$ref": "#/responses/BranchProtection" + // "404": + // "$ref": "#/responses/notFound" repo := ctx.Repo.Repository bpName := ctx.Params(":name") protectBranch, err := models.GetProtectedBranchBy(repo.ID, bpName) if err != nil { - ctx.Error(500, "GetProtectedBranchByID", err) + ctx.Error(http.StatusInternalServerError, "GetProtectedBranchByID", err) return } if protectBranch == nil || protectBranch.RepoID != repo.ID { @@ -475,15 +483,15 @@ func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtection // Reload from db to ensure get all whitelists bp, err := models.GetProtectedBranchByID(ctx.ParamsInt64(":id")) if err != nil { - ctx.Error(500, "GetProtectedBranchByID", err) + ctx.Error(http.StatusInternalServerError, "GetProtectedBranchByID", err) return } if bp == nil || bp.RepoID != ctx.Repo.Repository.ID { - ctx.Error(500, "New branch protection not found", err) + ctx.Error(http.StatusInternalServerError, "New branch protection not found", err) return } - ctx.JSON(200, convert.ToBranchProtection(bp)) + ctx.JSON(http.StatusOK, convert.ToBranchProtection(bp)) } // DeleteBranchProtection deletes a branch protection for a repo @@ -513,12 +521,14 @@ func DeleteBranchProtection(ctx *context.APIContext) { // responses: // "204": // "$ref": "#/responses/empty" + // "404": + // "$ref": "#/responses/notFound" repo := ctx.Repo.Repository bpName := ctx.Params(":name") bp, err := models.GetProtectedBranchBy(repo.ID, bpName) if err != nil { - ctx.Error(500, "GetProtectedBranchByID", err) + ctx.Error(http.StatusInternalServerError, "GetProtectedBranchByID", err) return } if bp == nil || bp.RepoID != repo.ID { @@ -531,5 +541,5 @@ func DeleteBranchProtection(ctx *context.APIContext) { return } - ctx.Status(204) + ctx.Status(http.StatusNoContent) } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 4aa0a56aa965..8e007cbcc3e3 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -1775,6 +1775,12 @@ "responses": { "200": { "$ref": "#/responses/BranchProtection" + }, + "403": { + "$ref": "#/responses/forbidden" + }, + "404": { + "$ref": "#/responses/notFound" } } } @@ -1816,6 +1822,9 @@ "responses": { "204": { "$ref": "#/responses/empty" + }, + "404": { + "$ref": "#/responses/notFound" } } } @@ -1857,6 +1866,9 @@ "responses": { "200": { "$ref": "#/responses/BranchProtection" + }, + "404": { + "$ref": "#/responses/notFound" } } }, @@ -1904,8 +1916,11 @@ } ], "responses": { - "204": { + "200": { "$ref": "#/responses/BranchProtection" + }, + "404": { + "$ref": "#/responses/notFound" } } } From 5d83df5c803374636b35c1fa795c87baa7becb9b Mon Sep 17 00:00:00 2001 From: David Svantesson Date: Sat, 18 Jan 2020 20:49:51 +0100 Subject: [PATCH 05/13] fix --- routers/api/v1/repo/branch.go | 14 ++++++------- templates/swagger/v1_json.tmpl | 38 ++++++++++++++++------------------ 2 files changed, 25 insertions(+), 27 deletions(-) diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 9ce90b33ad80..b09e4ffffad1 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -139,9 +139,9 @@ func GetBranchProtection(ctx *context.APIContext) { // description: name of the repo // type: string // required: true - // - name: id + // - name: name // in: path - // description: ID of the branch protection + // description: name of protected branch // type: integer // format: int64 // required: true @@ -346,9 +346,9 @@ func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtection // description: name of the repo // type: string // required: true - // - name: id + // - name: name // in: path - // description: ID of the branch protection + // description: name of protected branch // type: integer // format: int64 // required: true @@ -496,7 +496,7 @@ func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtection // DeleteBranchProtection deletes a branch protection for a repo func DeleteBranchProtection(ctx *context.APIContext) { - // swagger:operation DELETE /repos/{owner}/{repo}/branch_protections/{id} repository repoDeleteBranchProtection + // swagger:operation DELETE /repos/{owner}/{repo}/branch_protections/{name} repository repoDeleteBranchProtection // --- // summary: Delete a specific branch protection for the repository // produces: @@ -512,9 +512,9 @@ func DeleteBranchProtection(ctx *context.APIContext) { // description: name of the repo // type: string // required: true - // - name: id + // - name: name // in: path - // description: ID of the branch protection + // description: name of protected branch // type: integer // format: int64 // required: true diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 8e007cbcc3e3..7a9c0d5f5e3a 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -1785,16 +1785,16 @@ } } }, - "/repos/{owner}/{repo}/branch_protections/{id}": { - "delete": { + "/repos/{owner}/{repo}/branch_protections/{name}": { + "get": { "produces": [ "application/json" ], "tags": [ "repository" ], - "summary": "Delete a specific branch protection for the repository", - "operationId": "repoDeleteBranchProtection", + "summary": "Get a specific branch protection for the repository", + "operationId": "repoGetBranchProtection", "parameters": [ { "type": "string", @@ -1813,32 +1813,30 @@ { "type": "integer", "format": "int64", - "description": "ID of the branch protection", - "name": "id", + "description": "name of protected branch", + "name": "name", "in": "path", "required": true } ], "responses": { - "204": { - "$ref": "#/responses/empty" + "200": { + "$ref": "#/responses/BranchProtection" }, "404": { "$ref": "#/responses/notFound" } } - } - }, - "/repos/{owner}/{repo}/branch_protections/{name}": { - "get": { + }, + "delete": { "produces": [ "application/json" ], "tags": [ "repository" ], - "summary": "Get a specific branch protection for the repository", - "operationId": "repoGetBranchProtection", + "summary": "Delete a specific branch protection for the repository", + "operationId": "repoDeleteBranchProtection", "parameters": [ { "type": "string", @@ -1857,15 +1855,15 @@ { "type": "integer", "format": "int64", - "description": "ID of the branch protection", - "name": "id", + "description": "name of protected branch", + "name": "name", "in": "path", "required": true } ], "responses": { - "200": { - "$ref": "#/responses/BranchProtection" + "204": { + "$ref": "#/responses/empty" }, "404": { "$ref": "#/responses/notFound" @@ -1902,8 +1900,8 @@ { "type": "integer", "format": "int64", - "description": "ID of the branch protection", - "name": "id", + "description": "name of protected branch", + "name": "name", "in": "path", "required": true }, From 2234a5974840336b99680ef594473df8b7609af7 Mon Sep 17 00:00:00 2001 From: David Svantesson Date: Sat, 25 Jan 2020 15:41:38 +0100 Subject: [PATCH 06/13] Fix --- modules/convert/convert.go | 1 - modules/structs/repo_branch.go | 1 - routers/api/v1/repo/branch.go | 113 +++++++++++++++++++++++++-------- templates/swagger/v1_json.tmpl | 20 +++--- 4 files changed, 97 insertions(+), 38 deletions(-) diff --git a/modules/convert/convert.go b/modules/convert/convert.go index a45fea399b76..8fcf30db6df4 100644 --- a/modules/convert/convert.go +++ b/modules/convert/convert.go @@ -90,7 +90,6 @@ func ToBranchProtection(bp *models.ProtectedBranch) *api.BranchProtection { } return &api.BranchProtection{ - ID: bp.ID, BranchName: bp.BranchName, EnablePush: bp.CanPush, EnablePushWhitelist: bp.EnableWhitelist, diff --git a/modules/structs/repo_branch.go b/modules/structs/repo_branch.go index 3e5c8c2b2a50..fb2935485157 100644 --- a/modules/structs/repo_branch.go +++ b/modules/structs/repo_branch.go @@ -23,7 +23,6 @@ type Branch struct { // BranchProtection represents a branch protection for a repository type BranchProtection struct { - ID int64 `json:"id"` BranchName string `json:"branch_name"` EnablePush bool `json:"enable_push"` EnablePushWhitelist bool `json:"enable_push_whitelist"` diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index b09e4ffffad1..8b94d92e1512 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -142,8 +142,7 @@ func GetBranchProtection(ctx *context.APIContext) { // - name: name // in: path // description: name of protected branch - // type: integer - // format: int64 + // type: string // required: true // responses: // "200": @@ -233,6 +232,8 @@ func CreateBranchProtection(ctx *context.APIContext, form api.CreateBranchProtec // "$ref": "#/responses/forbidden" // "404": // "$ref": "#/responses/notFound" + // "422": + // "$ref": "#/responses/validationError" repo := ctx.Repo.Repository @@ -244,7 +245,7 @@ func CreateBranchProtection(ctx *context.APIContext, form api.CreateBranchProtec protectBranch, err := models.GetProtectedBranchBy(repo.ID, form.BranchName) if err != nil { - ctx.ServerError("GetProtectBranchOfRepoByName", err) + ctx.Error(http.StatusInternalServerError, "GetProtectBranchOfRepoByName", err) return } else if protectBranch != nil { ctx.Error(http.StatusForbidden, "Branch protection already exist", err) @@ -258,34 +259,63 @@ func CreateBranchProtection(ctx *context.APIContext, form api.CreateBranchProtec whitelistUsers, err := models.GetUserIDsByNames(form.PushWhitelistUsernames, false) if err != nil { - ctx.ServerError("GetUserIDsByNames", err) + if models.IsErrUserNotExist(err) { + ctx.Error(http.StatusUnprocessableEntity, "User does not exist", err) + return + } + ctx.Error(http.StatusInternalServerError, "GetUserIDsByNames", err) + return } mergeWhitelistUsers, err := models.GetUserIDsByNames(form.MergeWhitelistUsernames, false) if err != nil { - ctx.ServerError("GetUserIDsByNames", err) + if models.IsErrUserNotExist(err) { + ctx.Error(http.StatusUnprocessableEntity, "User does not exist", err) + return + } + ctx.Error(http.StatusInternalServerError, "GetUserIDsByNames", err) + return } approvalsWhitelistUsers, err := models.GetUserIDsByNames(form.ApprovalsWhitelistUsernames, false) if err != nil { - ctx.ServerError("GetUserIDsByNames", err) + if models.IsErrUserNotExist(err) { + ctx.Error(http.StatusUnprocessableEntity, "User does not exist", err) + return + } + ctx.Error(http.StatusInternalServerError, "GetUserIDsByNames", err) + return } var whitelistTeams, mergeWhitelistTeams, approvalsWhitelistTeams []int64 if repo.Owner.IsOrganization() { whitelistTeams, err = models.GetTeamIDsByNames(repo.OwnerID, form.PushWhitelistTeams, false) if err != nil { - ctx.ServerError("GetTeamIDsByNames", err) + if models.IsErrTeamNotExist(err) { + ctx.Error(http.StatusUnprocessableEntity, "Team does not exist", err) + return + } + ctx.Error(http.StatusInternalServerError, "GetTeamIDsByNames", err) + return } mergeWhitelistTeams, err = models.GetTeamIDsByNames(repo.OwnerID, form.MergeWhitelistTeams, false) if err != nil { - ctx.ServerError("GetTeamIDsByNames", err) + if models.IsErrTeamNotExist(err) { + ctx.Error(http.StatusUnprocessableEntity, "Team does not exist", err) + return + } + ctx.Error(http.StatusInternalServerError, "GetTeamIDsByNames", err) + return } approvalsWhitelistTeams, err = models.GetTeamIDsByNames(repo.OwnerID, form.ApprovalsWhitelistTeams, false) if err != nil { - ctx.ServerError("GetTeamIDsByNames", err) + if models.IsErrTeamNotExist(err) { + ctx.Error(http.StatusUnprocessableEntity, "Team does not exist", err) + return + } + ctx.Error(http.StatusInternalServerError, "GetTeamIDsByNames", err) + return } } protectBranch = &models.ProtectedBranch{ - ID: 0, RepoID: ctx.Repo.Repository.ID, BranchName: form.BranchName, CanPush: form.EnablePush, @@ -307,7 +337,7 @@ func CreateBranchProtection(ctx *context.APIContext, form api.CreateBranchProtec ApprovalsTeamIDs: approvalsWhitelistTeams, }) if err != nil { - ctx.ServerError("UpdateProtectBranch", err) + ctx.Error(http.StatusInternalServerError, "UpdateProtectBranch", err) return } @@ -349,8 +379,7 @@ func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtection // - name: name // in: path // description: name of protected branch - // type: integer - // format: int64 + // type: string // required: true // - name: body // in: body @@ -361,6 +390,8 @@ func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtection // "$ref": "#/responses/BranchProtection" // "404": // "$ref": "#/responses/notFound" + // "422": + // "$ref": "#/responses/validationError" repo := ctx.Repo.Repository bpName := ctx.Params(":name") @@ -400,6 +431,9 @@ func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtection } if form.EnableStatusCheck != nil { + protectBranch.EnableStatusCheck = *form.EnableStatusCheck + } + if protectBranch.EnableStatusCheck { protectBranch.StatusCheckContexts = form.StatusCheckContexts } @@ -415,7 +449,12 @@ func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtection if form.PushWhitelistUsernames != nil { whitelistUsers, err = models.GetUserIDsByNames(form.PushWhitelistUsernames, false) if err != nil { - ctx.ServerError("GetUserIDsByNames", err) + if models.IsErrUserNotExist(err) { + ctx.Error(http.StatusUnprocessableEntity, "User does not exist", err) + return + } + ctx.Error(http.StatusInternalServerError, "GetUserIDsByNames", err) + return } } else { whitelistUsers = protectBranch.WhitelistUserIDs @@ -424,7 +463,12 @@ func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtection if form.MergeWhitelistUsernames != nil { mergeWhitelistUsers, err = models.GetUserIDsByNames(form.MergeWhitelistUsernames, false) if err != nil { - ctx.ServerError("GetUserIDsByNames", err) + if models.IsErrUserNotExist(err) { + ctx.Error(http.StatusUnprocessableEntity, "User does not exist", err) + return + } + ctx.Error(http.StatusInternalServerError, "GetUserIDsByNames", err) + return } } else { mergeWhitelistUsers = protectBranch.MergeWhitelistUserIDs @@ -433,7 +477,12 @@ func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtection if form.ApprovalsWhitelistUsernames != nil { approvalsWhitelistUsers, err = models.GetUserIDsByNames(form.ApprovalsWhitelistUsernames, false) if err != nil { - ctx.ServerError("GetUserIDsByNames", err) + if models.IsErrUserNotExist(err) { + ctx.Error(http.StatusUnprocessableEntity, "User does not exist", err) + return + } + ctx.Error(http.StatusInternalServerError, "GetUserIDsByNames", err) + return } } else { approvalsWhitelistUsers = protectBranch.ApprovalsWhitelistUserIDs @@ -444,7 +493,12 @@ func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtection if form.PushWhitelistTeams != nil { whitelistTeams, err = models.GetTeamIDsByNames(repo.OwnerID, form.PushWhitelistTeams, false) if err != nil { - ctx.ServerError("GetTeamIDsByNames", err) + if models.IsErrTeamNotExist(err) { + ctx.Error(http.StatusUnprocessableEntity, "Team does not exist", err) + return + } + ctx.Error(http.StatusInternalServerError, "GetTeamIDsByNames", err) + return } } else { whitelistTeams = protectBranch.WhitelistTeamIDs @@ -452,7 +506,12 @@ func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtection if form.MergeWhitelistTeams != nil { mergeWhitelistTeams, err = models.GetTeamIDsByNames(repo.OwnerID, form.MergeWhitelistTeams, false) if err != nil { - ctx.ServerError("GetTeamIDsByNames", err) + if models.IsErrTeamNotExist(err) { + ctx.Error(http.StatusUnprocessableEntity, "Team does not exist", err) + return + } + ctx.Error(http.StatusInternalServerError, "GetTeamIDsByNames", err) + return } } else { mergeWhitelistTeams = protectBranch.MergeWhitelistTeamIDs @@ -460,7 +519,12 @@ func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtection if form.ApprovalsWhitelistTeams != nil { approvalsWhitelistTeams, err = models.GetTeamIDsByNames(repo.OwnerID, form.ApprovalsWhitelistTeams, false) if err != nil { - ctx.ServerError("GetTeamIDsByNames", err) + if models.IsErrTeamNotExist(err) { + ctx.Error(http.StatusUnprocessableEntity, "Team does not exist", err) + return + } + ctx.Error(http.StatusInternalServerError, "GetTeamIDsByNames", err) + return } } else { approvalsWhitelistTeams = protectBranch.ApprovalsWhitelistTeamIDs @@ -476,14 +540,14 @@ func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtection ApprovalsTeamIDs: approvalsWhitelistTeams, }) if err != nil { - ctx.ServerError("UpdateProtectBranch", err) + ctx.Error(http.StatusInternalServerError, "UpdateProtectBranch", err) return } // Reload from db to ensure get all whitelists - bp, err := models.GetProtectedBranchByID(ctx.ParamsInt64(":id")) + bp, err := models.GetProtectedBranchBy(repo.ID, bpName) if err != nil { - ctx.Error(http.StatusInternalServerError, "GetProtectedBranchByID", err) + ctx.Error(http.StatusInternalServerError, "GetProtectedBranchBy", err) return } if bp == nil || bp.RepoID != ctx.Repo.Repository.ID { @@ -515,8 +579,7 @@ func DeleteBranchProtection(ctx *context.APIContext) { // - name: name // in: path // description: name of protected branch - // type: integer - // format: int64 + // type: string // required: true // responses: // "204": @@ -537,7 +600,7 @@ func DeleteBranchProtection(ctx *context.APIContext) { } if err := ctx.Repo.Repository.DeleteProtectedBranch(bp.ID); err != nil { - ctx.ServerError("DeleteProtectedBranch", err) + ctx.Error(http.StatusInternalServerError, "DeleteProtectedBranch", err) return } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index cd1cbfd289ac..ea9eb0c9defd 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -1873,6 +1873,9 @@ }, "404": { "$ref": "#/responses/notFound" + }, + "422": { + "$ref": "#/responses/validationError" } } } @@ -1903,8 +1906,7 @@ "required": true }, { - "type": "integer", - "format": "int64", + "type": "string", "description": "name of protected branch", "name": "name", "in": "path", @@ -1945,8 +1947,7 @@ "required": true }, { - "type": "integer", - "format": "int64", + "type": "string", "description": "name of protected branch", "name": "name", "in": "path", @@ -1990,8 +1991,7 @@ "required": true }, { - "type": "integer", - "format": "int64", + "type": "string", "description": "name of protected branch", "name": "name", "in": "path", @@ -2011,6 +2011,9 @@ }, "404": { "$ref": "#/responses/notFound" + }, + "422": { + "$ref": "#/responses/validationError" } } } @@ -9641,11 +9644,6 @@ "type": "boolean", "x-go-name": "EnableStatusCheck" }, - "id": { - "type": "integer", - "format": "int64", - "x-go-name": "ID" - }, "merge_whitelist_teams": { "type": "array", "items": { From 606fbbc469f915673d32e086e04cfa038d2d8c86 Mon Sep 17 00:00:00 2001 From: David Svantesson Date: Sat, 25 Jan 2020 16:00:31 +0100 Subject: [PATCH 07/13] Add new branch protection options (BlockOnRejectedReviews, DismissStaleApprovals, RequireSignedCommits) --- modules/convert/convert.go | 3 +++ modules/structs/repo_branch.go | 9 +++++++++ routers/api/v1/repo/branch.go | 15 ++++++++++++++ templates/swagger/v1_json.tmpl | 36 ++++++++++++++++++++++++++++++++++ 4 files changed, 63 insertions(+) diff --git a/modules/convert/convert.go b/modules/convert/convert.go index 8fcf30db6df4..31b46bc7eb01 100644 --- a/modules/convert/convert.go +++ b/modules/convert/convert.go @@ -105,6 +105,9 @@ func ToBranchProtection(bp *models.ProtectedBranch) *api.BranchProtection { EnableApprovalsWhitelist: bp.EnableApprovalsWhitelist, ApprovalsWhitelistUsernames: approvalsWhitelistUsernames, ApprovalsWhitelistTeams: approvalsWhitelistTeams, + BlockOnRejectedReviews: bp.BlockOnRejectedReviews, + DismissStaleApprovals: bp.DismissStaleApprovals, + RequireSignedCommits: bp.RequireSignedCommits, Created: bp.CreatedUnix.AsTime(), Updated: bp.UpdatedUnix.AsTime(), } diff --git a/modules/structs/repo_branch.go b/modules/structs/repo_branch.go index fb2935485157..f8c929054889 100644 --- a/modules/structs/repo_branch.go +++ b/modules/structs/repo_branch.go @@ -38,6 +38,9 @@ type BranchProtection struct { EnableApprovalsWhitelist bool `json:"enable_approvals_whitelist"` ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"` ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"` + BlockOnRejectedReviews bool `json:"block_on_rejected_reviews"` + DismissStaleApprovals bool `json:"dismiss_stale_approvals"` + RequireSignedCommits bool `json:"require_signed_commits"` // swagger:strfmt date-time Created time.Time `json:"created_at"` // swagger:strfmt date-time @@ -61,6 +64,9 @@ type CreateBranchProtectionOption struct { EnableApprovalsWhitelist bool `json:"enable_approvals_whitelist"` ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"` ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"` + BlockOnRejectedReviews bool `json:"block_on_rejected_reviews"` + DismissStaleApprovals bool `json:"dismiss_stale_approvals"` + RequireSignedCommits bool `json:"require_signed_commits"` } // EditBranchProtectionOption options for editing a branch protection @@ -79,4 +85,7 @@ type EditBranchProtectionOption struct { EnableApprovalsWhitelist *bool `json:"enable_approvals_whitelist"` ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"` ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"` + BlockOnRejectedReviews *bool `json:"block_on_rejected_reviews"` + DismissStaleApprovals *bool `json:"dismiss_stale_approvals"` + RequireSignedCommits *bool `json:"require_signed_commits"` } diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 8b94d92e1512..546f3fb259e1 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -326,6 +326,9 @@ func CreateBranchProtection(ctx *context.APIContext, form api.CreateBranchProtec StatusCheckContexts: form.StatusCheckContexts, EnableApprovalsWhitelist: form.EnableApprovalsWhitelist, RequiredApprovals: requiredApprovals, + BlockOnRejectedReviews: form.BlockOnRejectedReviews, + DismissStaleApprovals: form.DismissStaleApprovals, + RequireSignedCommits: form.RequireSignedCommits, } err = models.UpdateProtectBranch(ctx.Repo.Repository, protectBranch, models.WhitelistOptions{ @@ -445,6 +448,18 @@ func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtection protectBranch.EnableApprovalsWhitelist = *form.EnableApprovalsWhitelist } + if form.BlockOnRejectedReviews != nil { + protectBranch.BlockOnRejectedReviews = *form.BlockOnRejectedReviews + } + + if form.DismissStaleApprovals != nil { + protectBranch.DismissStaleApprovals = *form.DismissStaleApprovals + } + + if form.RequireSignedCommits != nil { + protectBranch.RequireSignedCommits = *form.RequireSignedCommits + } + var whitelistUsers []int64 if form.PushWhitelistUsernames != nil { whitelistUsers, err = models.GetUserIDsByNames(form.PushWhitelistUsernames, false) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index ea9eb0c9defd..152790b59f27 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -9615,6 +9615,10 @@ }, "x-go-name": "ApprovalsWhitelistUsernames" }, + "block_on_rejected_reviews": { + "type": "boolean", + "x-go-name": "BlockOnRejectedReviews" + }, "branch_name": { "type": "string", "x-go-name": "BranchName" @@ -9624,6 +9628,10 @@ "format": "date-time", "x-go-name": "Created" }, + "dismiss_stale_approvals": { + "type": "boolean", + "x-go-name": "DismissStaleApprovals" + }, "enable_approvals_whitelist": { "type": "boolean", "x-go-name": "EnableApprovalsWhitelist" @@ -9676,6 +9684,10 @@ }, "x-go-name": "PushWhitelistUsernames" }, + "require_signed_commits": { + "type": "boolean", + "x-go-name": "RequireSignedCommits" + }, "required_approvals": { "type": "integer", "format": "int64", @@ -9919,10 +9931,18 @@ }, "x-go-name": "ApprovalsWhitelistUsernames" }, + "block_on_rejected_reviews": { + "type": "boolean", + "x-go-name": "BlockOnRejectedReviews" + }, "branch_name": { "type": "string", "x-go-name": "BranchName" }, + "dismiss_stale_approvals": { + "type": "boolean", + "x-go-name": "DismissStaleApprovals" + }, "enable_approvals_whitelist": { "type": "boolean", "x-go-name": "EnableApprovalsWhitelist" @@ -9975,6 +9995,10 @@ }, "x-go-name": "PushWhitelistUsernames" }, + "require_signed_commits": { + "type": "boolean", + "x-go-name": "RequireSignedCommits" + }, "required_approvals": { "type": "integer", "format": "int64", @@ -10691,6 +10715,14 @@ }, "x-go-name": "ApprovalsWhitelistUsernames" }, + "block_on_rejected_reviews": { + "type": "boolean", + "x-go-name": "BlockOnRejectedReviews" + }, + "dismiss_stale_approvals": { + "type": "boolean", + "x-go-name": "DismissStaleApprovals" + }, "enable_approvals_whitelist": { "type": "boolean", "x-go-name": "EnableApprovalsWhitelist" @@ -10743,6 +10775,10 @@ }, "x-go-name": "PushWhitelistUsernames" }, + "require_signed_commits": { + "type": "boolean", + "x-go-name": "RequireSignedCommits" + }, "required_approvals": { "type": "integer", "format": "int64", From ba86c20ee4369812fae6f7abad0b9cfa49be35ec Mon Sep 17 00:00:00 2001 From: David Svantesson Date: Sun, 26 Jan 2020 09:21:24 +0100 Subject: [PATCH 08/13] Do xorm query directly --- models/user.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/models/user.go b/models/user.go index a6d081b43284..edd33c3485c7 100644 --- a/models/user.go +++ b/models/user.go @@ -1371,11 +1371,11 @@ func GetMaileableUsersByIDs(ids []int64) ([]*User, error) { // GetUserNamesByIDs returns usernames for all resolved users from a list of Ids. func GetUserNamesByIDs(ids []int64) ([]string, error) { - ous, err := GetUsersByIDs(ids) - unames := make([]string, 0, len(ous)) - for _, u := range ous { - unames = append(unames, u.Name) - } + unames := make([]string, 0, len(ids)) + err := x.In("id", ids). + Asc("name"). + Cols("name"). + Find(&unames) return unames, err } From 2d86ed10c09b05d0d1c701fa9394c6381dcf90bd Mon Sep 17 00:00:00 2001 From: David Svantesson Date: Sun, 26 Jan 2020 11:55:04 +0100 Subject: [PATCH 09/13] fix xorm GetUserNamesByIDs --- models/user.go | 1 + 1 file changed, 1 insertion(+) diff --git a/models/user.go b/models/user.go index edd33c3485c7..7506fbd3874f 100644 --- a/models/user.go +++ b/models/user.go @@ -1373,6 +1373,7 @@ func GetMaileableUsersByIDs(ids []int64) ([]*User, error) { func GetUserNamesByIDs(ids []int64) ([]string, error) { unames := make([]string, 0, len(ids)) err := x.In("id", ids). + Table("user"). Asc("name"). Cols("name"). Find(&unames) From 92583c3e666de514ad379f62cb13658e80c6a733 Mon Sep 17 00:00:00 2001 From: David Svantesson Date: Sun, 26 Jan 2020 14:49:49 +0100 Subject: [PATCH 10/13] Add some tests --- integrations/api_branch_test.go | 68 +++++++++++++++++++++++++++++++++ routers/api/v1/repo/branch.go | 2 +- 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/integrations/api_branch_test.go b/integrations/api_branch_test.go index 037a42deec6a..ac87c11c9c9f 100644 --- a/integrations/api_branch_test.go +++ b/integrations/api_branch_test.go @@ -30,6 +30,54 @@ func testAPIGetBranch(t *testing.T, branchName string, exists bool) { assert.EqualValues(t, branchName, branch.Name) } +func testAPIGetBranchProtection(t *testing.T, branchName string, expectedHTTPStatus int) { + session := loginUser(t, "user2") + token := getTokenForLoggedInUser(t, session) + req := NewRequestf(t, "GET", "/api/v1/repos/user2/repo1/branch_protections/%s?token=%s", branchName, token) + resp := session.MakeRequest(t, req, expectedHTTPStatus) + + if resp.Code == 200 { + var branchProtection api.BranchProtection + DecodeJSON(t, resp, &branchProtection) + assert.EqualValues(t, branchName, branchProtection.BranchName) + } +} + +func testAPICreateBranchProtection(t *testing.T, branchName string, expectedHTTPStatus int) { + session := loginUser(t, "user2") + token := getTokenForLoggedInUser(t, session) + req := NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/repo1/branch_protections?token="+token, &api.BranchProtection{ + BranchName: branchName, + }) + resp := session.MakeRequest(t, req, expectedHTTPStatus) + + if resp.Code == 200 { + var branchProtection api.BranchProtection + DecodeJSON(t, resp, &branchProtection) + assert.EqualValues(t, branchName, branchProtection.BranchName) + } +} + +func testAPIEditBranchProtection(t *testing.T, branchName string, body *api.BranchProtection, expectedHTTPStatus int) { + session := loginUser(t, "user2") + token := getTokenForLoggedInUser(t, session) + req := NewRequestWithJSON(t, "PATCH", "/api/v1/repos/user2/repo1/branch_protections/"+branchName+"?token="+token, body) + resp := session.MakeRequest(t, req, expectedHTTPStatus) + + if resp.Code == 200 { + var branchProtection api.BranchProtection + DecodeJSON(t, resp, &branchProtection) + assert.EqualValues(t, branchName, branchProtection.BranchName) + } +} + +func testAPIDeleteBranchProtection(t *testing.T, branchName string, expectedHTTPStatus int) { + session := loginUser(t, "user2") + token := getTokenForLoggedInUser(t, session) + req := NewRequestf(t, "DELETE", "/api/v1/repos/user2/repo1/branch_protections/%s?token=%s", branchName, token) + session.MakeRequest(t, req, expectedHTTPStatus) +} + func TestAPIGetBranch(t *testing.T) { for _, test := range []struct { BranchName string @@ -43,3 +91,23 @@ func TestAPIGetBranch(t *testing.T) { testAPIGetBranch(t, test.BranchName, test.Exists) } } + +func TestAPIBranchProtection(t *testing.T) { + defer prepareTestEnv(t)() + + // Branch protection only on branch that exist + testAPICreateBranchProtection(t, "master/doesnotexist", http.StatusNotFound) + // Get branch protection on branch that exist but not branch protection + testAPIGetBranchProtection(t, "master", http.StatusNotFound) + + testAPICreateBranchProtection(t, "master", http.StatusOK) + // Can only create once + testAPICreateBranchProtection(t, "master", http.StatusForbidden) + + testAPIGetBranchProtection(t, "master", http.StatusOK) + testAPIEditBranchProtection(t, "master", &api.BranchProtection{ + EnablePush: true, + }, http.StatusOK) + + testAPIDeleteBranchProtection(t, "master", http.StatusNoContent) +} diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 546f3fb259e1..59b6949fbf84 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -248,7 +248,7 @@ func CreateBranchProtection(ctx *context.APIContext, form api.CreateBranchProtec ctx.Error(http.StatusInternalServerError, "GetProtectBranchOfRepoByName", err) return } else if protectBranch != nil { - ctx.Error(http.StatusForbidden, "Branch protection already exist", err) + ctx.Error(http.StatusForbidden, "Create branch protection", "Branch protection already exist") return } From ed778e660cba1ed73e44cd92920feb2e2f1ae0bc Mon Sep 17 00:00:00 2001 From: David Svantesson Date: Mon, 27 Jan 2020 17:45:42 +0100 Subject: [PATCH 11/13] Improved GetTeamNamesByID --- models/org_team.go | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/models/org_team.go b/models/org_team.go index 33123fcf4c21..f8013d12c6e9 100644 --- a/models/org_team.go +++ b/models/org_team.go @@ -597,17 +597,14 @@ func GetTeamNamesByID(teamIDs []int64) ([]string, error) { return []string{}, nil } - teams := make([]*Team, 0, len(teamIDs)) - - err := x.In("id", teamIDs). + var teamNames []string + err := x.Table("team"). + Select("lower_name"). + In("id", teamIDs). Asc("name"). - Find(&teams) + Find(&teamNames) - teamnames := make([]string, 0, len(teams)) - for _, t := range teams { - teamnames = append(teamnames, t.LowerName) - } - return teamnames, err + return teamNames, err } // UpdateTeam updates information of team. From 3ccc769ea308016b91f434845bb4e5566c132587 Mon Sep 17 00:00:00 2001 From: David Svantesson Date: Tue, 4 Feb 2020 07:52:05 +0100 Subject: [PATCH 12/13] http status created for CreateBranchProtection --- routers/api/v1/repo/branch.go | 4 ++-- templates/swagger/v1_json.tmpl | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 59b6949fbf84..fccfc2bfe1aa 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -226,7 +226,7 @@ func CreateBranchProtection(ctx *context.APIContext, form api.CreateBranchProtec // schema: // "$ref": "#/definitions/CreateBranchProtectionOption" // responses: - // "200": + // "201": // "$ref": "#/responses/BranchProtection" // "403": // "$ref": "#/responses/forbidden" @@ -355,7 +355,7 @@ func CreateBranchProtection(ctx *context.APIContext, form api.CreateBranchProtec return } - ctx.JSON(http.StatusOK, convert.ToBranchProtection(bp)) + ctx.JSON(http.StatusCreated, convert.ToBranchProtection(bp)) } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 152790b59f27..16cad86938af 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -1865,7 +1865,7 @@ } ], "responses": { - "200": { + "201": { "$ref": "#/responses/BranchProtection" }, "403": { From 9b285d3eb64d8c886359a91ce9b56c73faab47ca Mon Sep 17 00:00:00 2001 From: David Svantesson Date: Tue, 4 Feb 2020 08:15:38 +0100 Subject: [PATCH 13/13] Correct status code in integration test --- integrations/api_branch_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integrations/api_branch_test.go b/integrations/api_branch_test.go index ac87c11c9c9f..3fe7f23229d6 100644 --- a/integrations/api_branch_test.go +++ b/integrations/api_branch_test.go @@ -51,7 +51,7 @@ func testAPICreateBranchProtection(t *testing.T, branchName string, expectedHTTP }) resp := session.MakeRequest(t, req, expectedHTTPStatus) - if resp.Code == 200 { + if resp.Code == 201 { var branchProtection api.BranchProtection DecodeJSON(t, resp, &branchProtection) assert.EqualValues(t, branchName, branchProtection.BranchName) @@ -100,7 +100,7 @@ func TestAPIBranchProtection(t *testing.T) { // Get branch protection on branch that exist but not branch protection testAPIGetBranchProtection(t, "master", http.StatusNotFound) - testAPICreateBranchProtection(t, "master", http.StatusOK) + testAPICreateBranchProtection(t, "master", http.StatusCreated) // Can only create once testAPICreateBranchProtection(t, "master", http.StatusForbidden)