From 1dfa28ffa557720d164a351783be64c9a47a0adb Mon Sep 17 00:00:00 2001 From: qwerty287 <80460567+qwerty287@users.noreply.github.com> Date: Thu, 29 Sep 2022 04:27:20 +0200 Subject: [PATCH 1/5] Add API endpoint to get changed files of a PR (#21177) This adds an api endpoint `/files` to PRs that allows to get a list of changed files. built upon #18228, reviews there are included closes https://github.com/go-gitea/gitea/issues/654 Co-authored-by: Anton Bracke Co-authored-by: 6543 <6543@obermui.de> Co-authored-by: wxiaoguang --- models/fixtures/pull_request.yml | 4 +- modules/convert/convert.go | 34 +++++++ modules/structs/pull.go | 13 +++ routers/api/v1/api.go | 1 + routers/api/v1/repo/pull.go | 136 +++++++++++++++++++++++++ routers/api/v1/swagger/repo.go | 22 ++++ templates/swagger/v1_json.tmpl | 155 +++++++++++++++++++++++++++++ tests/integration/api_pull_test.go | 48 ++++++++- 8 files changed, 407 insertions(+), 6 deletions(-) diff --git a/models/fixtures/pull_request.yml b/models/fixtures/pull_request.yml index d45baa711c6e..165437f0328d 100644 --- a/models/fixtures/pull_request.yml +++ b/models/fixtures/pull_request.yml @@ -60,8 +60,8 @@ head_repo_id: 1 base_repo_id: 1 head_branch: pr-to-update - base_branch: branch1 - merge_base: 1234567890abcdef + base_branch: branch2 + merge_base: 985f0301dba5e7b34be866819cd15ad3d8f508ee has_merged: false - diff --git a/modules/convert/convert.go b/modules/convert/convert.go index c62b4303ed27..0e6756307713 100644 --- a/modules/convert/convert.go +++ b/modules/convert/convert.go @@ -27,6 +27,7 @@ import ( "code.gitea.io/gitea/modules/log" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" + "code.gitea.io/gitea/services/gitdiff" webhook_service "code.gitea.io/gitea/services/webhook" ) @@ -414,3 +415,36 @@ func ToLFSLock(l *git_model.LFSLock) *api.LFSLock { }, } } + +// ToChangedFile convert a gitdiff.DiffFile to api.ChangedFile +func ToChangedFile(f *gitdiff.DiffFile, repo *repo_model.Repository, commit string) *api.ChangedFile { + status := "changed" + if f.IsDeleted { + status = "deleted" + } else if f.IsCreated { + status = "added" + } else if f.IsRenamed && f.Type == gitdiff.DiffFileCopy { + status = "copied" + } else if f.IsRenamed && f.Type == gitdiff.DiffFileRename { + status = "renamed" + } else if f.Addition == 0 && f.Deletion == 0 { + status = "unchanged" + } + + file := &api.ChangedFile{ + Filename: f.GetDiffFileName(), + Status: status, + Additions: f.Addition, + Deletions: f.Deletion, + Changes: f.Addition + f.Deletion, + HTMLURL: fmt.Sprint(repo.HTMLURL(), "/src/commit/", commit, "/", util.PathEscapeSegments(f.GetDiffFileName())), + ContentsURL: fmt.Sprint(repo.APIURL(), "/contents/", util.PathEscapeSegments(f.GetDiffFileName()), "?ref=", commit), + RawURL: fmt.Sprint(repo.HTMLURL(), "/raw/commit/", commit, "/", util.PathEscapeSegments(f.GetDiffFileName())), + } + + if status == "rename" { + file.PreviousFilename = f.OldName + } + + return file +} diff --git a/modules/structs/pull.go b/modules/structs/pull.go index b63b3edfd30a..f627241b2630 100644 --- a/modules/structs/pull.go +++ b/modules/structs/pull.go @@ -95,3 +95,16 @@ type EditPullRequestOption struct { RemoveDeadline *bool `json:"unset_due_date"` AllowMaintainerEdit *bool `json:"allow_maintainer_edit"` } + +// ChangedFile store information about files affected by the pull request +type ChangedFile struct { + Filename string `json:"filename"` + PreviousFilename string `json:"previous_filename,omitempty"` + Status string `json:"status"` + Additions int `json:"additions"` + Deletions int `json:"deletions"` + Changes int `json:"changes"` + HTMLURL string `json:"html_url,omitempty"` + ContentsURL string `json:"contents_url,omitempty"` + RawURL string `json:"raw_url,omitempty"` +} diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 6a98121b73a5..0d11674aa997 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1002,6 +1002,7 @@ func Routes(ctx gocontext.Context) *web.Route { m.Get(".{diffType:diff|patch}", repo.DownloadPullDiffOrPatch) m.Post("/update", reqToken(), repo.UpdatePullRequest) m.Get("/commits", repo.GetPullRequestCommits) + m.Get("/files", repo.GetPullRequestFiles) m.Combo("/merge").Get(repo.IsPullRequestMerged). Post(reqToken(), mustNotBeArchived, bind(forms.MergePullRequestForm{}), repo.MergePullRequest). Delete(reqToken(), mustNotBeArchived, repo.CancelScheduledAutoMerge) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index dda05203df85..2cf30e7c47b8 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -26,6 +26,7 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/notification" + "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/web" @@ -33,6 +34,7 @@ import ( asymkey_service "code.gitea.io/gitea/services/asymkey" "code.gitea.io/gitea/services/automerge" "code.gitea.io/gitea/services/forms" + "code.gitea.io/gitea/services/gitdiff" issue_service "code.gitea.io/gitea/services/issue" pull_service "code.gitea.io/gitea/services/pull" repo_service "code.gitea.io/gitea/services/repository" @@ -1323,3 +1325,137 @@ func GetPullRequestCommits(ctx *context.APIContext) { ctx.JSON(http.StatusOK, &apiCommits) } + +// GetPullRequestFiles gets all changed files associated with a given PR +func GetPullRequestFiles(ctx *context.APIContext) { + // swagger:operation GET /repos/{owner}/{repo}/pulls/{index}/files repository repoGetPullRequestFiles + // --- + // summary: Get changed files for a pull request + // 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 + // - name: skip-to + // in: query + // description: skip to given file + // type: string + // - name: whitespace + // in: query + // description: whitespace behavior + // type: string + // enum: [ignore-all, ignore-change, ignore-eol, show-all] + // - name: page + // in: query + // description: page number of results to return (1-based) + // type: integer + // - name: limit + // in: query + // description: page size of results + // type: integer + // responses: + // "200": + // "$ref": "#/responses/ChangedFileList" + // "404": + // "$ref": "#/responses/notFound" + + pr, err := issues_model.GetPullRequestByIndex(ctx, ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) + if err != nil { + if issues_model.IsErrPullRequestNotExist(err) { + ctx.NotFound() + } else { + ctx.Error(http.StatusInternalServerError, "GetPullRequestByIndex", err) + } + return + } + + if err := pr.LoadBaseRepo(); err != nil { + ctx.InternalServerError(err) + return + } + + if err := pr.LoadHeadRepo(); err != nil { + ctx.InternalServerError(err) + return + } + + baseGitRepo := ctx.Repo.GitRepo + + var prInfo *git.CompareInfo + if pr.HasMerged { + prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.MergeBase, pr.GetGitRefName(), true, false) + } else { + prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitRefName(), true, false) + } + if err != nil { + ctx.ServerError("GetCompareInfo", err) + return + } + + headCommitID, err := baseGitRepo.GetRefCommitID(pr.GetGitRefName()) + if err != nil { + ctx.ServerError("GetRefCommitID", err) + return + } + + startCommitID := prInfo.MergeBase + endCommitID := headCommitID + + maxLines, maxFiles := setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles + + diff, err := gitdiff.GetDiff(baseGitRepo, + &gitdiff.DiffOptions{ + BeforeCommitID: startCommitID, + AfterCommitID: endCommitID, + SkipTo: ctx.FormString("skip-to"), + MaxLines: maxLines, + MaxLineCharacters: setting.Git.MaxGitDiffLineCharacters, + MaxFiles: maxFiles, + WhitespaceBehavior: gitdiff.GetWhitespaceFlag(ctx.FormString("whitespace")), + }) + if err != nil { + ctx.ServerError("GetDiff", err) + return + } + + listOptions := utils.GetListOptions(ctx) + + totalNumberOfFiles := diff.NumFiles + totalNumberOfPages := int(math.Ceil(float64(totalNumberOfFiles) / float64(listOptions.PageSize))) + + start, end := listOptions.GetStartEnd() + + if end > totalNumberOfFiles { + end = totalNumberOfFiles + } + + apiFiles := make([]*api.ChangedFile, 0, end-start) + for i := start; i < end; i++ { + apiFiles = append(apiFiles, convert.ToChangedFile(diff.Files[i], pr.HeadRepo, endCommitID)) + } + + ctx.SetLinkHeader(totalNumberOfFiles, listOptions.PageSize) + ctx.SetTotalCountHeader(int64(totalNumberOfFiles)) + + ctx.RespHeader().Set("X-Page", strconv.Itoa(listOptions.Page)) + ctx.RespHeader().Set("X-PerPage", strconv.Itoa(listOptions.PageSize)) + ctx.RespHeader().Set("X-PageCount", strconv.Itoa(totalNumberOfPages)) + ctx.RespHeader().Set("X-HasMore", strconv.FormatBool(listOptions.Page < totalNumberOfPages)) + ctx.AppendAccessControlExposeHeaders("X-Page", "X-PerPage", "X-PageCount", "X-HasMore") + + ctx.JSON(http.StatusOK, &apiFiles) +} diff --git a/routers/api/v1/swagger/repo.go b/routers/api/v1/swagger/repo.go index 3522e242764e..642b1b7b9102 100644 --- a/routers/api/v1/swagger/repo.go +++ b/routers/api/v1/swagger/repo.go @@ -254,6 +254,28 @@ type swaggerCommitList struct { Body []api.Commit `json:"body"` } +// ChangedFileList +// swagger:response ChangedFileList +type swaggerChangedFileList struct { + // The current page + Page int `json:"X-Page"` + + // Commits per page + PerPage int `json:"X-PerPage"` + + // Total commit count + Total int `json:"X-Total"` + + // Total number of pages + PageCount int `json:"X-PageCount"` + + // True if there is another page + HasMore bool `json:"X-HasMore"` + + // in: body + Body []api.ChangedFile `json:"body"` +} + // Note // swagger:response Note type swaggerNote struct { diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 47100450f945..52553e2e8991 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -8019,6 +8019,80 @@ } } }, + "/repos/{owner}/{repo}/pulls/{index}/files": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Get changed files for a pull request", + "operationId": "repoGetPullRequestFiles", + "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 + }, + { + "type": "string", + "description": "skip to given file", + "name": "skip-to", + "in": "query" + }, + { + "enum": [ + "ignore-all", + "ignore-change", + "ignore-eol", + "show-all" + ], + "type": "string", + "description": "whitespace behavior", + "name": "whitespace", + "in": "query" + }, + { + "type": "integer", + "description": "page number of results to return (1-based)", + "name": "page", + "in": "query" + }, + { + "type": "integer", + "description": "page size of results", + "name": "limit", + "in": "query" + } + ], + "responses": { + "200": { + "$ref": "#/responses/ChangedFileList" + }, + "404": { + "$ref": "#/responses/notFound" + } + } + } + }, "/repos/{owner}/{repo}/pulls/{index}/merge": { "get": { "produces": [ @@ -13715,6 +13789,52 @@ }, "x-go-package": "code.gitea.io/gitea/modules/structs" }, + "ChangedFile": { + "description": "ChangedFile store information about files affected by the pull request", + "type": "object", + "properties": { + "additions": { + "type": "integer", + "format": "int64", + "x-go-name": "Additions" + }, + "changes": { + "type": "integer", + "format": "int64", + "x-go-name": "Changes" + }, + "contents_url": { + "type": "string", + "x-go-name": "ContentsURL" + }, + "deletions": { + "type": "integer", + "format": "int64", + "x-go-name": "Deletions" + }, + "filename": { + "type": "string", + "x-go-name": "Filename" + }, + "html_url": { + "type": "string", + "x-go-name": "HTMLURL" + }, + "previous_filename": { + "type": "string", + "x-go-name": "PreviousFilename" + }, + "raw_url": { + "type": "string", + "x-go-name": "RawURL" + }, + "status": { + "type": "string", + "x-go-name": "Status" + } + }, + "x-go-package": "code.gitea.io/gitea/modules/structs" + }, "CombinedStatus": { "description": "CombinedStatus holds the combined state of several statuses for a single commit", "type": "object", @@ -19173,6 +19293,41 @@ } } }, + "ChangedFileList": { + "description": "ChangedFileList", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/ChangedFile" + } + }, + "headers": { + "X-HasMore": { + "type": "boolean", + "description": "True if there is another page" + }, + "X-Page": { + "type": "integer", + "format": "int64", + "description": "The current page" + }, + "X-PageCount": { + "type": "integer", + "format": "int64", + "description": "Total number of pages" + }, + "X-PerPage": { + "type": "integer", + "format": "int64", + "description": "Commits per page" + }, + "X-Total": { + "type": "integer", + "format": "int64", + "description": "Total commit count" + } + } + }, "CombinedStatus": { "description": "CombinedStatus", "schema": { diff --git a/tests/integration/api_pull_test.go b/tests/integration/api_pull_test.go index 032912a07360..8ce92f3d4a62 100644 --- a/tests/integration/api_pull_test.go +++ b/tests/integration/api_pull_test.go @@ -6,6 +6,7 @@ package integration import ( "fmt" + "io/ioutil" "net/http" "testing" @@ -27,15 +28,35 @@ func TestAPIViewPulls(t *testing.T) { repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) - session := loginUser(t, "user2") - token := getTokenForLoggedInUser(t, session) - req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/pulls?state=all&token="+token, owner.Name, repo.Name) - resp := session.MakeRequest(t, req, http.StatusOK) + ctx := NewAPITestContext(t, "user2", repo.Name) + + req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/pulls?state=all&token="+ctx.Token, owner.Name, repo.Name) + resp := ctx.Session.MakeRequest(t, req, http.StatusOK) var pulls []*api.PullRequest DecodeJSON(t, resp, &pulls) expectedLen := unittest.GetCount(t, &issues_model.Issue{RepoID: repo.ID}, unittest.Cond("is_pull = ?", true)) assert.Len(t, pulls, expectedLen) + + pull := pulls[0] + if assert.EqualValues(t, 5, pull.ID) { + resp = ctx.Session.MakeRequest(t, NewRequest(t, "GET", pull.DiffURL), http.StatusOK) + _, err := ioutil.ReadAll(resp.Body) + assert.NoError(t, err) + // TODO: use diff to generate stats to test against + + t.Run(fmt.Sprintf("APIGetPullFiles_%d", pull.ID), + doAPIGetPullFiles(ctx, pull, func(t *testing.T, files []*api.ChangedFile) { + if assert.Len(t, files, 1) { + assert.EqualValues(t, "File-WoW", files[0].Filename) + assert.EqualValues(t, "", files[0].PreviousFilename) + assert.EqualValues(t, 1, files[0].Additions) + assert.EqualValues(t, 1, files[0].Changes) + assert.EqualValues(t, 0, files[0].Deletions) + assert.EqualValues(t, "added", files[0].Status) + } + })) + } } // TestAPIMergePullWIP ensures that we can't merge a WIP pull request @@ -183,3 +204,22 @@ func TestAPIEditPull(t *testing.T) { }) session.MakeRequest(t, req, http.StatusNotFound) } + +func doAPIGetPullFiles(ctx APITestContext, pr *api.PullRequest, callback func(*testing.T, []*api.ChangedFile)) func(*testing.T) { + return func(t *testing.T) { + url := fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/files?token=%s", ctx.Username, ctx.Reponame, pr.Index, ctx.Token) + + req := NewRequest(t, http.MethodGet, url) + if ctx.ExpectedCode == 0 { + ctx.ExpectedCode = http.StatusOK + } + resp := ctx.Session.MakeRequest(t, req, ctx.ExpectedCode) + + files := make([]*api.ChangedFile, 0, 1) + DecodeJSON(t, resp, &files) + + if callback != nil { + callback(t, files) + } + } +} From b7309b8ccb16f6303ae300b755baef9f9713d457 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Thu, 29 Sep 2022 05:27:33 +0200 Subject: [PATCH 2/5] Add name field for org api (#21270) related #21205 The field `UserName` is not really usefull for an organization. This adds a second `Name` field. The [GitHub API](https://docs.github.com/en/rest/orgs/orgs#get-an-organization) uses `name` too. `UserName` should be deprecated then. --- modules/convert/convert.go | 1 + modules/structs/org.go | 4 +++- templates/swagger/v1_json.tmpl | 5 +++++ tests/integration/api_admin_org_test.go | 2 +- tests/integration/api_org_test.go | 6 +++--- tests/integration/api_user_orgs_test.go | 4 ++++ 6 files changed, 17 insertions(+), 5 deletions(-) diff --git a/modules/convert/convert.go b/modules/convert/convert.go index 0e6756307713..af759cb9388b 100644 --- a/modules/convert/convert.go +++ b/modules/convert/convert.go @@ -296,6 +296,7 @@ func ToOrganization(org *organization.Organization) *api.Organization { return &api.Organization{ ID: org.ID, AvatarURL: org.AsUser().AvatarLink(), + Name: org.Name, UserName: org.Name, FullName: org.FullName, Description: org.Description, diff --git a/modules/structs/org.go b/modules/structs/org.go index d8bd59e1ecb7..1e98c59ba49c 100644 --- a/modules/structs/org.go +++ b/modules/structs/org.go @@ -7,7 +7,7 @@ package structs // Organization represents an organization type Organization struct { ID int64 `json:"id"` - UserName string `json:"username"` + Name string `json:"name"` FullName string `json:"full_name"` AvatarURL string `json:"avatar_url"` Description string `json:"description"` @@ -15,6 +15,8 @@ type Organization struct { Location string `json:"location"` Visibility string `json:"visibility"` RepoAdminChangeTeamAccess bool `json:"repo_admin_change_team_access"` + // deprecated + UserName string `json:"username"` } // OrganizationPermissions list different users permissions on an organization diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 52553e2e8991..b0e94b8bb5b3 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -17326,11 +17326,16 @@ "type": "string", "x-go-name": "Location" }, + "name": { + "type": "string", + "x-go-name": "Name" + }, "repo_admin_change_team_access": { "type": "boolean", "x-go-name": "RepoAdminChangeTeamAccess" }, "username": { + "description": "deprecated", "type": "string", "x-go-name": "UserName" }, diff --git a/tests/integration/api_admin_org_test.go b/tests/integration/api_admin_org_test.go index 720f6fc6b645..a8770db4ca40 100644 --- a/tests/integration/api_admin_org_test.go +++ b/tests/integration/api_admin_org_test.go @@ -37,7 +37,7 @@ func TestAPIAdminOrgCreate(t *testing.T) { var apiOrg api.Organization DecodeJSON(t, resp, &apiOrg) - assert.Equal(t, org.UserName, apiOrg.UserName) + assert.Equal(t, org.UserName, apiOrg.Name) assert.Equal(t, org.FullName, apiOrg.FullName) assert.Equal(t, org.Description, apiOrg.Description) assert.Equal(t, org.Website, apiOrg.Website) diff --git a/tests/integration/api_org_test.go b/tests/integration/api_org_test.go index 4b8c5c97a8a5..16e53d6b8158 100644 --- a/tests/integration/api_org_test.go +++ b/tests/integration/api_org_test.go @@ -38,7 +38,7 @@ func TestAPIOrgCreate(t *testing.T) { var apiOrg api.Organization DecodeJSON(t, resp, &apiOrg) - assert.Equal(t, org.UserName, apiOrg.UserName) + assert.Equal(t, org.UserName, apiOrg.Name) assert.Equal(t, org.FullName, apiOrg.FullName) assert.Equal(t, org.Description, apiOrg.Description) assert.Equal(t, org.Website, apiOrg.Website) @@ -54,7 +54,7 @@ func TestAPIOrgCreate(t *testing.T) { req = NewRequestf(t, "GET", "/api/v1/orgs/%s?token=%s", org.UserName, token) resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiOrg) - assert.EqualValues(t, org.UserName, apiOrg.UserName) + assert.EqualValues(t, org.UserName, apiOrg.Name) req = NewRequestf(t, "GET", "/api/v1/orgs/%s/repos?token=%s", org.UserName, token) resp = MakeRequest(t, req, http.StatusOK) @@ -94,7 +94,7 @@ func TestAPIOrgEdit(t *testing.T) { var apiOrg api.Organization DecodeJSON(t, resp, &apiOrg) - assert.Equal(t, "user3", apiOrg.UserName) + assert.Equal(t, "user3", apiOrg.Name) assert.Equal(t, org.FullName, apiOrg.FullName) assert.Equal(t, org.Description, apiOrg.Description) assert.Equal(t, org.Website, apiOrg.Website) diff --git a/tests/integration/api_user_orgs_test.go b/tests/integration/api_user_orgs_test.go index 622dfdcf21e0..c28bf391eb3a 100644 --- a/tests/integration/api_user_orgs_test.go +++ b/tests/integration/api_user_orgs_test.go @@ -32,6 +32,7 @@ func TestUserOrgs(t *testing.T) { assert.Equal(t, []*api.Organization{ { ID: 17, + Name: user17.Name, UserName: user17.Name, FullName: user17.FullName, AvatarURL: user17.AvatarLink(), @@ -42,6 +43,7 @@ func TestUserOrgs(t *testing.T) { }, { ID: 3, + Name: user3.Name, UserName: user3.Name, FullName: user3.FullName, AvatarURL: user3.AvatarLink(), @@ -99,6 +101,7 @@ func TestMyOrgs(t *testing.T) { assert.Equal(t, []*api.Organization{ { ID: 17, + Name: user17.Name, UserName: user17.Name, FullName: user17.FullName, AvatarURL: user17.AvatarLink(), @@ -109,6 +112,7 @@ func TestMyOrgs(t *testing.T) { }, { ID: 3, + Name: user3.Name, UserName: user3.Name, FullName: user3.FullName, AvatarURL: user3.AvatarLink(), From 1d3095b71849d7084a26638af831e284d942cb43 Mon Sep 17 00:00:00 2001 From: Alexander Shimchik Date: Thu, 29 Sep 2022 15:36:29 +0300 Subject: [PATCH 3/5] Check if email is used when updating user (#21289) Fix #21075 When updating user data should check if email is used by other users --- models/user/user.go | 17 +++++++++++------ models/user/user_test.go | 16 ++++++++++++++++ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/models/user/user.go b/models/user/user.go index 32484a487f50..a3c10c2492dd 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -893,14 +893,19 @@ func UpdateUser(ctx context.Context, u *User, changePrimaryEmail bool, cols ...s if err != nil { return err } - if !has { - // 1. Update old primary email - if _, err = e.Where("uid=? AND is_primary=?", u.ID, true).Cols("is_primary").Update(&EmailAddress{ - IsPrimary: false, - }); err != nil { - return err + if has && emailAddress.UID != u.ID { + return ErrEmailAlreadyUsed{ + Email: u.Email, } + } + // 1. Update old primary email + if _, err = e.Where("uid=? AND is_primary=?", u.ID, true).Cols("is_primary").Update(&EmailAddress{ + IsPrimary: false, + }); err != nil { + return err + } + if !has { emailAddress.Email = u.Email emailAddress.UID = u.ID emailAddress.IsActivated = true diff --git a/models/user/user_test.go b/models/user/user_test.go index 848c978a9ba5..678d6c186cf1 100644 --- a/models/user/user_test.go +++ b/models/user/user_test.go @@ -302,10 +302,26 @@ func TestUpdateUser(t *testing.T) { user = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) assert.True(t, user.KeepActivityPrivate) + newEmail := "new_" + user.Email + user.Email = newEmail + assert.NoError(t, user_model.UpdateUser(db.DefaultContext, user, true)) + user = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + assert.Equal(t, newEmail, user.Email) + user.Email = "no mail@mail.org" assert.Error(t, user_model.UpdateUser(db.DefaultContext, user, true)) } +func TestUpdateUserEmailAlreadyUsed(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + user3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}) + + user2.Email = user3.Email + err := user_model.UpdateUser(db.DefaultContext, user2, true) + assert.True(t, user_model.IsErrEmailAlreadyUsed(err)) +} + func TestNewUserRedirect(t *testing.T) { // redirect to a completely new name assert.NoError(t, unittest.PrepareTestDatabase()) From 3b6a7e5c8a994836d034adde4277b50ade020d1b Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 30 Sep 2022 01:20:22 +0800 Subject: [PATCH 4/5] Fix the hook related FAQ contents (#21297) Follows https://github.com/go-gitea/gitea/issues/21129#issuecomment-1260802986 * https://github.com/go-gitea/gitea/issues/21129#issuecomment-1260802986 A lot of users are asking similar questions. The old content in FAQ doesn't seem to be related to the problem. --- docs/content/doc/help/faq.en-us.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/content/doc/help/faq.en-us.md b/docs/content/doc/help/faq.en-us.md index f71cd734a50a..a59abe833568 100644 --- a/docs/content/doc/help/faq.en-us.md +++ b/docs/content/doc/help/faq.en-us.md @@ -214,13 +214,13 @@ Our translations are currently crowd-sourced on our [Crowdin project](https://cr Whether you want to change a translation or add a new one, it will need to be there as all translations are overwritten in our CI via the Crowdin integration. -## Hooks aren't running +## Push Hook / Webhook aren't running -If Gitea is not running hooks, a common cause is incorrect setup of SSH keys. +If you can push but can't see push activities on the home dashboard, or the push doesn't trigger webhook, there are a few possibilities: -See [SSH Issues](#ssh-issues) for more information. - -You can also try logging into the administration panel and running the `Resynchronize pre-receive, update and post-receive hooks of all repositories.` option. +1. The git hooks are out of sync: run "Resynchronize pre-receive, update and post-receive hooks of all repositories" on the site admin panel +2. The git repositories (and hooks) are stored on some filesystems (ex: mounted by NAS) which don't support script execution, make sure the filesystem supports `chmod a+x any-script` +3. If you are using docker, make sure Docker Server (not the client) >= 20.10.6 ## SSH issues From 08609d439d81f65c4f691609acaa29c1c7e96757 Mon Sep 17 00:00:00 2001 From: qwerty287 <80460567+qwerty287@users.noreply.github.com> Date: Thu, 29 Sep 2022 21:09:14 +0200 Subject: [PATCH 5/5] Add pages to view watched repos and subscribed issues/PRs (#17156) Adds GitHub-like pages to view watched repos and subscribed issues/PRs This is my second try to fix this, but it is better than the first since it doesn't uses a filter option which could be slow when accessing `/issues` or `/pulls` and it shows both pulls and issues (the first try is #17053). Closes #16111 Replaces and closes #17053 ![Screenshot](https://user-images.githubusercontent.com/80460567/134782937-3112f7da-425a-45b6-9511-5c9695aee896.png) Co-authored-by: Lauris BH Co-authored-by: 6543 <6543@obermui.de> Co-authored-by: wxiaoguang --- models/issues/issue.go | 35 +++ options/locale/locale_en-US.ini | 3 + routers/web/user/notification.go | 213 +++++++++++++++++- routers/web/web.go | 2 + templates/base/head_navbar.tmpl | 4 + .../notification_subscriptions.tmpl | 79 +++++++ 6 files changed, 334 insertions(+), 2 deletions(-) create mode 100644 templates/user/notification/notification_subscriptions.tmpl diff --git a/models/issues/issue.go b/models/issues/issue.go index 5bdb60f7c08c..49bc229c6bd0 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -1186,6 +1186,7 @@ type IssuesOptions struct { //nolint PosterID int64 MentionedID int64 ReviewRequestedID int64 + SubscriberID int64 MilestoneIDs []int64 ProjectID int64 ProjectBoardID int64 @@ -1299,6 +1300,10 @@ func (opts *IssuesOptions) setupSessionNoLimit(sess *xorm.Session) { applyReviewRequestedCondition(sess, opts.ReviewRequestedID) } + if opts.SubscriberID > 0 { + applySubscribedCondition(sess, opts.SubscriberID) + } + if len(opts.MilestoneIDs) > 0 { sess.In("issue.milestone_id", opts.MilestoneIDs) } @@ -1463,6 +1468,36 @@ func applyReviewRequestedCondition(sess *xorm.Session, reviewRequestedID int64) reviewRequestedID, ReviewTypeApprove, ReviewTypeReject, ReviewTypeRequest, reviewRequestedID) } +func applySubscribedCondition(sess *xorm.Session, subscriberID int64) *xorm.Session { + return sess.And( + builder. + NotIn("issue.id", + builder.Select("issue_id"). + From("issue_watch"). + Where(builder.Eq{"is_watching": false, "user_id": subscriberID}), + ), + ).And( + builder.Or( + builder.In("issue.id", builder. + Select("issue_id"). + From("issue_watch"). + Where(builder.Eq{"is_watching": true, "user_id": subscriberID}), + ), + builder.In("issue.id", builder. + Select("issue_id"). + From("comment"). + Where(builder.Eq{"poster_id": subscriberID}), + ), + builder.Eq{"issue.poster_id": subscriberID}, + builder.In("issue.repo_id", builder. + Select("id"). + From("watch"). + Where(builder.Eq{"user_id": subscriberID, "mode": true}), + ), + ), + ) +} + // CountIssuesByRepo map from repoID to number of issues matching the options func CountIssuesByRepo(opts *IssuesOptions) (map[int64]int64, error) { e := db.GetEngine(db.DefaultContext) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 991ebf344f82..1dba1d71d8ff 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -3034,6 +3034,9 @@ pin = Pin notification mark_as_read = Mark as read mark_as_unread = Mark as unread mark_all_as_read = Mark all as read +subscriptions = Subscriptions +watching = Watching +no_subscriptions = No subscriptions [gpg] default_key=Signed with default key diff --git a/routers/web/user/notification.go b/routers/web/user/notification.go index 5e8142cec721..b4753a603e8a 100644 --- a/routers/web/user/notification.go +++ b/routers/web/user/notification.go @@ -13,16 +13,23 @@ import ( "strings" activities_model "code.gitea.io/gitea/models/activities" + "code.gitea.io/gitea/models/db" + issues_model "code.gitea.io/gitea/models/issues" + repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" + issue_service "code.gitea.io/gitea/services/issue" + pull_service "code.gitea.io/gitea/services/pull" ) const ( - tplNotification base.TplName = "user/notification/notification" - tplNotificationDiv base.TplName = "user/notification/notification_div" + tplNotification base.TplName = "user/notification/notification" + tplNotificationDiv base.TplName = "user/notification/notification_div" + tplNotificationSubscriptions base.TplName = "user/notification/notification_subscriptions" ) // GetNotificationCount is the middleware that sets the notification count in the context @@ -197,6 +204,208 @@ func NotificationPurgePost(c *context.Context) { c.Redirect(setting.AppSubURL+"/notifications", http.StatusSeeOther) } +// NotificationSubscriptions returns the list of subscribed issues +func NotificationSubscriptions(c *context.Context) { + page := c.FormInt("page") + if page < 1 { + page = 1 + } + + sortType := c.FormString("sort") + c.Data["SortType"] = sortType + + state := c.FormString("state") + if !util.IsStringInSlice(state, []string{"all", "open", "closed"}, true) { + state = "all" + } + c.Data["State"] = state + var showClosed util.OptionalBool + switch state { + case "all": + showClosed = util.OptionalBoolNone + case "closed": + showClosed = util.OptionalBoolTrue + case "open": + showClosed = util.OptionalBoolFalse + } + + var issueTypeBool util.OptionalBool + issueType := c.FormString("issueType") + switch issueType { + case "issues": + issueTypeBool = util.OptionalBoolFalse + case "pulls": + issueTypeBool = util.OptionalBoolTrue + default: + issueTypeBool = util.OptionalBoolNone + } + c.Data["IssueType"] = issueType + + var labelIDs []int64 + selectedLabels := c.FormString("labels") + c.Data["Labels"] = selectedLabels + if len(selectedLabels) > 0 && selectedLabels != "0" { + var err error + labelIDs, err = base.StringsToInt64s(strings.Split(selectedLabels, ",")) + if err != nil { + c.ServerError("StringsToInt64s", err) + return + } + } + + count, err := issues_model.CountIssues(&issues_model.IssuesOptions{ + SubscriberID: c.Doer.ID, + IsClosed: showClosed, + IsPull: issueTypeBool, + LabelIDs: labelIDs, + }) + if err != nil { + c.ServerError("CountIssues", err) + return + } + issues, err := issues_model.Issues(&issues_model.IssuesOptions{ + ListOptions: db.ListOptions{ + PageSize: setting.UI.IssuePagingNum, + Page: page, + }, + SubscriberID: c.Doer.ID, + SortType: sortType, + IsClosed: showClosed, + IsPull: issueTypeBool, + LabelIDs: labelIDs, + }) + if err != nil { + c.ServerError("Issues", err) + return + } + + commitStatuses, lastStatus, err := pull_service.GetIssuesAllCommitStatus(c, issues) + if err != nil { + c.ServerError("GetIssuesAllCommitStatus", err) + return + } + c.Data["CommitLastStatus"] = lastStatus + c.Data["CommitStatuses"] = commitStatuses + c.Data["Issues"] = issues + + c.Data["IssueRefEndNames"], c.Data["IssueRefURLs"] = issue_service.GetRefEndNamesAndURLs(issues, "") + + commitStatus, err := pull_service.GetIssuesLastCommitStatus(c, issues) + if err != nil { + c.ServerError("GetIssuesLastCommitStatus", err) + return + } + c.Data["CommitStatus"] = commitStatus + + issueList := issues_model.IssueList(issues) + approvalCounts, err := issueList.GetApprovalCounts(c) + if err != nil { + c.ServerError("ApprovalCounts", err) + return + } + c.Data["ApprovalCounts"] = func(issueID int64, typ string) int64 { + counts, ok := approvalCounts[issueID] + if !ok || len(counts) == 0 { + return 0 + } + reviewTyp := issues_model.ReviewTypeApprove + if typ == "reject" { + reviewTyp = issues_model.ReviewTypeReject + } else if typ == "waiting" { + reviewTyp = issues_model.ReviewTypeRequest + } + for _, count := range counts { + if count.Type == reviewTyp { + return count.Count + } + } + return 0 + } + + c.Data["Status"] = 1 + c.Data["Title"] = c.Tr("notification.subscriptions") + + // redirect to last page if request page is more than total pages + pager := context.NewPagination(int(count), setting.UI.IssuePagingNum, page, 5) + if pager.Paginater.Current() < page { + c.Redirect(fmt.Sprintf("/notifications/subscriptions?page=%d", pager.Paginater.Current())) + return + } + pager.AddParam(c, "sort", "SortType") + pager.AddParam(c, "state", "State") + c.Data["Page"] = pager + + c.HTML(http.StatusOK, tplNotificationSubscriptions) +} + +// NotificationWatching returns the list of watching repos +func NotificationWatching(c *context.Context) { + page := c.FormInt("page") + if page < 1 { + page = 1 + } + + var orderBy db.SearchOrderBy + c.Data["SortType"] = c.FormString("sort") + switch c.FormString("sort") { + case "newest": + orderBy = db.SearchOrderByNewest + case "oldest": + orderBy = db.SearchOrderByOldest + case "recentupdate": + orderBy = db.SearchOrderByRecentUpdated + case "leastupdate": + orderBy = db.SearchOrderByLeastUpdated + case "reversealphabetically": + orderBy = db.SearchOrderByAlphabeticallyReverse + case "alphabetically": + orderBy = db.SearchOrderByAlphabetically + case "moststars": + orderBy = db.SearchOrderByStarsReverse + case "feweststars": + orderBy = db.SearchOrderByStars + case "mostforks": + orderBy = db.SearchOrderByForksReverse + case "fewestforks": + orderBy = db.SearchOrderByForks + default: + c.Data["SortType"] = "recentupdate" + orderBy = db.SearchOrderByRecentUpdated + } + + repos, count, err := repo_model.SearchRepository(&repo_model.SearchRepoOptions{ + ListOptions: db.ListOptions{ + PageSize: setting.UI.User.RepoPagingNum, + Page: page, + }, + Actor: c.Doer, + Keyword: c.FormTrim("q"), + OrderBy: orderBy, + Private: c.IsSigned, + WatchedByID: c.Doer.ID, + Collaborate: util.OptionalBoolFalse, + TopicOnly: c.FormBool("topic"), + IncludeDescription: setting.UI.SearchRepoDescription, + }) + if err != nil { + c.ServerError("ErrSearchRepository", err) + return + } + total := int(count) + c.Data["Total"] = total + c.Data["Repos"] = repos + + // redirect to last page if request page is more than total pages + pager := context.NewPagination(total, setting.UI.User.RepoPagingNum, page, 5) + pager.SetDefaultParams(c) + c.Data["Page"] = pager + + c.Data["Status"] = 2 + c.Data["Title"] = c.Tr("notification.watching") + + c.HTML(http.StatusOK, tplNotificationSubscriptions) +} + // NewAvailable returns the notification counts func NewAvailable(ctx *context.Context) { ctx.JSON(http.StatusOK, structs.NotificationCount{New: activities_model.CountUnread(ctx, ctx.Doer.ID)}) diff --git a/routers/web/web.go b/routers/web/web.go index 1852ecc2e24f..acce07189185 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1269,6 +1269,8 @@ func RegisterRoutes(m *web.Route) { m.Group("/notifications", func() { m.Get("", user.Notifications) + m.Get("/subscriptions", user.NotificationSubscriptions) + m.Get("/watching", user.NotificationWatching) m.Post("/status", user.NotificationStatusPost) m.Post("/purge", user.NotificationPurgePost) m.Get("/new", user.NewAvailable) diff --git a/templates/base/head_navbar.tmpl b/templates/base/head_navbar.tmpl index 8cd3b0a4ae0e..12837ebefedf 100644 --- a/templates/base/head_navbar.tmpl +++ b/templates/base/head_navbar.tmpl @@ -171,6 +171,10 @@ {{.locale.Tr "your_starred"}} {{end}} + + {{svg "octicon-bell"}} + {{.locale.Tr "notification.subscriptions"}} + {{svg "octicon-tools"}} {{.locale.Tr "your_settings"}} diff --git a/templates/user/notification/notification_subscriptions.tmpl b/templates/user/notification/notification_subscriptions.tmpl new file mode 100644 index 000000000000..aa89c12ddedd --- /dev/null +++ b/templates/user/notification/notification_subscriptions.tmpl @@ -0,0 +1,79 @@ +{{template "base/head" .}} +
+
+
+ + {{.locale.Tr "notification.subscriptions"}} + + + {{.locale.Tr "notification.watching"}} + +
+
+ {{if eq .Status 1}} + + {{if eq (len .Issues) 0}} +
+ {{.locale.Tr "notification.no_subscriptions"}} + {{else}} + {{template "shared/issuelist" mergeinto . "listType" "dashboard"}} + {{end}} + {{else}} + {{template "explore/repo_search" .}} + {{template "explore/repo_list" .}} + {{template "base/paginate" .}} + {{end}} +
+
+
+{{template "base/footer" .}}