- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 6.2k
 
Implement update branch API #32433
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement update branch API #32433
Changes from all commits
d8dbf6a
              591580a
              577b3dd
              b388d01
              6d221ed
              6a82204
              83e3528
              d906662
              db93cfa
              952523e
              af4e23d
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -386,6 +386,77 @@ func ListBranches(ctx *context.APIContext) { | |
| ctx.JSON(http.StatusOK, apiBranches) | ||
| } | ||
| 
     | 
||
| // UpdateBranch updates a repository's branch. | ||
| func UpdateBranch(ctx *context.APIContext) { | ||
| // swagger:operation PATCH /repos/{owner}/{repo}/branches/{branch} repository repoUpdateBranch | ||
| // --- | ||
| // summary: Update a branch | ||
| // 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: branch | ||
| // in: path | ||
| // description: name of the branch | ||
| // type: string | ||
| // required: true | ||
| // - name: body | ||
| // in: body | ||
| // schema: | ||
| // "$ref": "#/definitions/UpdateBranchRepoOption" | ||
| // responses: | ||
| // "204": | ||
| // "$ref": "#/responses/empty" | ||
| // "403": | ||
| // "$ref": "#/responses/forbidden" | ||
| // "404": | ||
| // "$ref": "#/responses/notFound" | ||
| // "422": | ||
| // "$ref": "#/responses/validationError" | ||
| 
     | 
||
| opt := web.GetForm(ctx).(*api.UpdateBranchRepoOption) | ||
| 
     | 
||
| oldName := ctx.PathParam("*") | ||
| repo := ctx.Repo.Repository | ||
| 
     | 
||
| if repo.IsEmpty { | ||
| ctx.Error(http.StatusNotFound, "", "Git Repository is empty.") | ||
| return | ||
| } | ||
| 
     | 
||
| if repo.IsMirror { | ||
| ctx.Error(http.StatusForbidden, "", "Git Repository is a mirror.") | ||
| return | ||
| } | ||
| 
         
      Comment on lines
    
      +433
     to 
      +441
    
   
  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe these checks can be done in  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this a blocked request? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we can put that for another PRs. This PR is focused on adding the API endpoint. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I originally found those checks in the other branch API implementations and believed it made sense to include them in here. I do agree that this logic should be moved to the service layer. To me, the presentation layer (i.e. the API and web routes handlers) should just be concerned with presenting the response given what the service layer provides. From my experience it does look like we have sprinkled application logic into our middleware and our web/API route handlers, so it does become confusing when considering where it should be added. But I also agree with doing this in a separate PR just to keeps our changes atomic (since I would also wan't to add service layer tests for these checks). I would be more than happy to do this once this one is merged.  | 
||
| 
     | 
||
| msg, err := repo_service.RenameBranch(ctx, repo, ctx.Doer, ctx.Repo.GitRepo, oldName, opt.Name) | ||
| if err != nil { | ||
| ctx.Error(http.StatusInternalServerError, "RenameBranch", err) | ||
| return | ||
| } | ||
| if msg == "target_exist" { | ||
| ctx.Error(http.StatusUnprocessableEntity, "", "Cannot rename a branch using the same name or rename to a branch that already exists.") | ||
| return | ||
| } | ||
| if msg == "from_not_exist" { | ||
| ctx.Error(http.StatusNotFound, "", "Branch doesn't exist.") | ||
| return | ||
| } | ||
| 
     | 
||
| ctx.Status(http.StatusNoContent) | ||
| } | ||
| 
     | 
||
| // GetBranchProtection gets a branch protection | ||
| func GetBranchProtection(ctx *context.APIContext) { | ||
| // swagger:operation GET /repos/{owner}/{repo}/branch_protections/{name} repository repoGetBranchProtection | ||
| 
          
            
          
           | 
    ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.