From f15f4e5e1f2673d11ba955083dee1df1955dcc13 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Mon, 3 Aug 2020 23:57:10 +0800 Subject: [PATCH 1/3] [API] Add update pr headBranch api Signed-off-by: a1012112796 <1012112796@qq.com> --- integrations/pull_update_test.go | 14 +++-- routers/api/v1/api.go | 1 + routers/api/v1/repo/pull.go | 94 ++++++++++++++++++++++++++++++++ routers/repo/pull.go | 2 +- templates/swagger/v1_json.tmpl | 50 +++++++++++++++++ 5 files changed, 154 insertions(+), 7 deletions(-) diff --git a/integrations/pull_update_test.go b/integrations/pull_update_test.go index 484390001c8b..2dc966316e6c 100644 --- a/integrations/pull_update_test.go +++ b/integrations/pull_update_test.go @@ -5,7 +5,7 @@ package integrations import ( - "fmt" + "net/http" "net/url" "testing" "time" @@ -19,7 +19,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestPullUpdate(t *testing.T) { +func TestAPIPullUpdate(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { //Create PR to test user := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) @@ -31,17 +31,19 @@ func TestPullUpdate(t *testing.T) { assert.NoError(t, err) assert.EqualValues(t, 1, diffCount.Behind) assert.EqualValues(t, 1, diffCount.Ahead) + assert.NoError(t, pr.LoadBaseRepo()) + assert.NoError(t, pr.LoadIssue()) - message := fmt.Sprintf("Merge branch '%s' into %s", pr.BaseBranch, pr.HeadBranch) - err = pull_service.Update(pr, user, message) - assert.NoError(t, err) + session := loginUser(t, "user2") + token := getTokenForLoggedInUser(t, session) + req := NewRequestf(t, "POST", "/api/v1/repos/%s/%s/pulls/%d/update?token="+token, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, pr.Issue.Index) + session.MakeRequest(t, req, http.StatusOK) //Test GetDiverging after update diffCount, err = pull_service.GetDiverging(pr) assert.NoError(t, err) assert.EqualValues(t, 0, diffCount.Behind) assert.EqualValues(t, 2, diffCount.Ahead) - }) } diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index b03f547a6bc7..36d4710942ee 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -800,6 +800,7 @@ func RegisterRoutes(m *macaron.Macaron) { Patch(reqToken(), reqRepoWriter(models.UnitTypePullRequests), bind(api.EditPullRequestOption{}), repo.EditPullRequest) m.Get(".diff", repo.DownloadPullDiff) m.Get(".patch", repo.DownloadPullPatch) + m.Post("/update", repo.UpdatePullRequest) m.Combo("/merge").Get(repo.IsPullRequestMerged). Post(reqToken(), mustNotBeArchived, bind(auth.MergePullRequestForm{}), repo.MergePullRequest) m.Group("/reviews", func() { diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 5acbb9e29709..0f0868ac8ada 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -968,3 +968,97 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) return headUser, headRepo, headGitRepo, compareInfo, baseBranch, headBranch } + +// UpdatePullRequest merge PR's baseBranch into headBranch +func UpdatePullRequest(ctx *context.APIContext) { + // swagger:operation POST /repos/{owner}/{repo}/pulls/{index}/update repository repoUpdatePullRequest + // --- + // summary: Merge PR's baseBranch into headBranch + // 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: index + // in: path + // description: index of the pull request to get + // type: integer + // format: int64 + // required: true + // responses: + // "200": + // "$ref": "#/responses/empty" + // "404": + // "$ref": "#/responses/notFound" + // "405": + // "$ref": "#/responses/empty" + // "409": + // "$ref": "#/responses/error" + + pr, err := models.GetPullRequestByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) + if err != nil { + if models.IsErrPullRequestNotExist(err) { + ctx.NotFound() + } else { + ctx.Error(http.StatusInternalServerError, "GetPullRequestByIndex", err) + } + return + } + + if pr.HasMerged { + ctx.Error(http.StatusNotFound, "UpdatePullRequest", err) + return + } + + if err = pr.LoadIssue(); err != nil { + ctx.Error(http.StatusInternalServerError, "LoadIssue", err) + return + } + + if pr.Issue.IsClosed { + ctx.Error(http.StatusNotFound, "UpdatePullRequest", err) + return + } + + if err = pr.LoadBaseRepo(); err != nil { + ctx.Error(http.StatusInternalServerError, "LoadBaseRepo", err) + return + } + if err = pr.LoadHeadRepo(); err != nil { + ctx.Error(http.StatusInternalServerError, "LoadHeadRepo", err) + return + } + + allowedUpdate, err := pull_service.IsUserAllowedToUpdate(pr, ctx.User) + if err != nil { + ctx.Error(http.StatusInternalServerError, "IsUserAllowedToMerge", err) + return + } + + if !allowedUpdate { + ctx.Status(http.StatusMethodNotAllowed) + return + } + + // default merge commit message + message := fmt.Sprintf("Merge branch '%s' into %s", pr.BaseBranch, pr.HeadBranch) + + if err = pull_service.Update(pr, ctx.User, message); err != nil { + if models.IsErrMergeConflicts(err) { + ctx.Error(http.StatusConflict, "Update", "merge failed because of conflict") + return + } + ctx.Error(http.StatusInternalServerError, "pull_service.Update", err) + return + } + + ctx.Status(http.StatusOK) +} diff --git a/routers/repo/pull.go b/routers/repo/pull.go index ebc4439dda79..d91590a356c4 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -666,7 +666,7 @@ func ViewPullFiles(ctx *context.Context) { ctx.HTML(200, tplPullFiles) } -// UpdatePullRequest merge master into PR +// UpdatePullRequest merge PR's baseBranch into headBranch func UpdatePullRequest(ctx *context.Context) { issue := checkPullInfo(ctx) if ctx.Written() { diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 28a33fb3d322..7941b5adc5a7 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -7285,6 +7285,56 @@ } } }, + "/repos/{owner}/{repo}/pulls/{index}/update": { + "post": { + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Merge PR's baseBranch into headBranch", + "operationId": "repoUpdatePullRequest", + "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": "index of the pull request to get", + "name": "index", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "$ref": "#/responses/empty" + }, + "404": { + "$ref": "#/responses/notFound" + }, + "405": { + "$ref": "#/responses/empty" + }, + "409": { + "$ref": "#/responses/error" + } + } + } + }, "/repos/{owner}/{repo}/raw/{filepath}": { "get": { "produces": [ From f402996c9a33dade51c270003ba1f492f7fb12c2 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Tue, 4 Aug 2020 17:40:41 +0800 Subject: [PATCH 2/3] add reqToken --- routers/api/v1/api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 36d4710942ee..b36393bb2634 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -800,7 +800,7 @@ func RegisterRoutes(m *macaron.Macaron) { Patch(reqToken(), reqRepoWriter(models.UnitTypePullRequests), bind(api.EditPullRequestOption{}), repo.EditPullRequest) m.Get(".diff", repo.DownloadPullDiff) m.Get(".patch", repo.DownloadPullPatch) - m.Post("/update", repo.UpdatePullRequest) + m.Post("/update", reqToken(), repo.UpdatePullRequest) m.Combo("/merge").Get(repo.IsPullRequestMerged). Post(reqToken(), mustNotBeArchived, bind(auth.MergePullRequestForm{}), repo.MergePullRequest) m.Group("/reviews", func() { From 6e37d6a76741097703b1ba7f6672b6044d2e3eac Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Wed, 5 Aug 2020 00:11:20 +0800 Subject: [PATCH 3/3] change return error status type, Thanks @6543 --- routers/api/v1/repo/pull.go | 12 +++++++----- templates/swagger/v1_json.tmpl | 9 ++++++--- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 0f0868ac8ada..5fc0cd8cfb52 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -996,12 +996,14 @@ func UpdatePullRequest(ctx *context.APIContext) { // responses: // "200": // "$ref": "#/responses/empty" + // "403": + // "$ref": "#/responses/forbidden" // "404": // "$ref": "#/responses/notFound" - // "405": - // "$ref": "#/responses/empty" // "409": // "$ref": "#/responses/error" + // "422": + // "$ref": "#/responses/validationError" pr, err := models.GetPullRequestByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { @@ -1014,7 +1016,7 @@ func UpdatePullRequest(ctx *context.APIContext) { } if pr.HasMerged { - ctx.Error(http.StatusNotFound, "UpdatePullRequest", err) + ctx.Error(http.StatusUnprocessableEntity, "UpdatePullRequest", err) return } @@ -1024,7 +1026,7 @@ func UpdatePullRequest(ctx *context.APIContext) { } if pr.Issue.IsClosed { - ctx.Error(http.StatusNotFound, "UpdatePullRequest", err) + ctx.Error(http.StatusUnprocessableEntity, "UpdatePullRequest", err) return } @@ -1044,7 +1046,7 @@ func UpdatePullRequest(ctx *context.APIContext) { } if !allowedUpdate { - ctx.Status(http.StatusMethodNotAllowed) + ctx.Status(http.StatusForbidden) return } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 0c5a7adeea10..d5e5c86cd8b7 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -7323,14 +7323,17 @@ "200": { "$ref": "#/responses/empty" }, + "403": { + "$ref": "#/responses/forbidden" + }, "404": { "$ref": "#/responses/notFound" }, - "405": { - "$ref": "#/responses/empty" - }, "409": { "$ref": "#/responses/error" + }, + "422": { + "$ref": "#/responses/validationError" } } }