From 55532061c83d38d33ef48bdc5eeac0f652844e8a Mon Sep 17 00:00:00 2001 From: sebastian-sauer Date: Fri, 28 Jul 2023 21:18:12 +0200 Subject: [PATCH] Add commits dropdown in PR files view and allow commit by commit review (#25528) This PR adds a new dropdown to select a commit or a commit range (shift-click like github) of a Pull Request. After selection of a commit only the changes of this commit will be shown. When selecting a range of commits the diff of this range is shown. This allows to review a PR commit by commit or by viewing only commit ranges. The "Show changes since your last review" mechanism github uses is implemented, too. When reviewing a single commit or a commit range the "Viewed" functionality is disabled. ## Screenshots ### The commit dropdown ![image](https://github.com/go-gitea/gitea/assets/51889757/0db3ae62-1272-436c-be64-4730c5d611e3) ### Selecting a commit range ![image](https://github.com/go-gitea/gitea/assets/51889757/ad81eedb-8437-42b0-8073-2d940c25fe8f) ### Show changes of a single commit only ![image](https://github.com/go-gitea/gitea/assets/51889757/6b1a113b-73ef-4ecc-adf6-bc2340bb8f97) ### Show changes of a commit range ![image](https://github.com/go-gitea/gitea/assets/51889757/6401b358-cd66-4c09-8baa-6cf6177f23a7) Fixes https://github.com/go-gitea/gitea/issues/20989 Fixes https://github.com/go-gitea/gitea/issues/19263 --------- Co-authored-by: silverwind Co-authored-by: KN4CK3R Co-authored-by: wxiaoguang Co-authored-by: delvh --- models/fixtures/issue.yml | 17 + models/fixtures/pull_request.yml | 13 + models/fixtures/repo_unit.yml | 30 ++ models/fixtures/repository.yml | 31 ++ models/fixtures/user.yml | 2 +- models/issues/issue_test.go | 2 +- models/issues/review_list.go | 2 +- models/repo/repo_list_test.go | 6 +- options/locale/locale_en-US.ini | 7 + routers/web/repo/pull.go | 116 ++++++- routers/web/user/home_test.go | 2 +- routers/web/web.go | 10 +- services/pull/pull.go | 71 +++++ templates/repo/diff/box.tmpl | 24 +- templates/repo/diff/new_review.tmpl | 4 +- .../user2/commitsonpr.git/HEAD | 1 + .../user2/commitsonpr.git/config | 4 + .../user2/commitsonpr.git/description | 1 + .../user2/commitsonpr.git/info/exclude | 6 + .../user2/commitsonpr.git/info/refs | 3 + .../user2/commitsonpr.git/logs/HEAD | 1 + .../commitsonpr.git/logs/refs/heads/branch1 | 1 + .../commitsonpr.git/logs/refs/heads/main | 1 + .../0a/6dda431c72a6a4aac05b98e319972a1a55e01c | Bin 0 -> 32 bytes .../0c/396a509b64fd4e2e55649d100b86e8b96cc0e5 | Bin 0 -> 32 bytes .../10/0ef49565829e7bd83057d2dab88f58b00db831 | Bin 0 -> 271 bytes .../10/9ab1c0b84e088d7edcf018379518b49361f285 | Bin 0 -> 32 bytes .../19/78192d98bb1b65e11c2cf37da854fbf94bffd6 | Bin 0 -> 162 bytes .../1e/67d753ac1f9097eff26f9d33eb80182344b72c | Bin 0 -> 87 bytes .../23/576dd018294e476c06e569b6b0f170d0558705 | 2 + .../28/16bffda09c0f23775ea4be279de004d28a3803 | Bin 0 -> 245 bytes .../35/f03b5e176ee6d24c86b5cca7009a5b0ba2a026 | Bin 0 -> 32 bytes .../38/cdad2e40c989aabab3f2d0a27faf0f7be617d5 | Bin 0 -> 167 bytes .../3e/64625bd6eb5bcba69ac97de6c8f507402df861 | Bin 0 -> 162 bytes .../4b/860706d3eec5858324d4ba00db0423ca4cbf50 | Bin 0 -> 54 bytes .../4c/a8bcaf27e28504df7bf996819665986b01c847 | Bin 0 -> 163 bytes .../52/84ca7f5757816e67c098224a8367aa2544222d | Bin 0 -> 193 bytes .../53/9a24812705f77484568e6ad7db84764c1903c8 | Bin 0 -> 32 bytes .../70/8605e8984e7fb9be58818e0e6d9f21bcefd63e | Bin 0 -> 33 bytes .../74/7ddb3506a4fa04a7747808eb56ae16f9e933dc | Bin 0 -> 162 bytes .../7e/d1d42eda9110676d5c3a7721965d6ed1afe83c | Bin 0 -> 324 bytes .../81/1d46c7e518f4f180afb862c0db5cb8c80529ce | 2 + .../83/2d33e438d2b4a86fba81cb67b32d1d61a828cb | Bin 0 -> 32 bytes .../83/7d5c8125633d7d258f93b998e867eab0145520 | 3 + .../87/cdc1333f5f117a92f3cef78ebe0301114b3610 | 2 + .../92/70b08497106eaa65fce8aa91f37c4780f76909 | Bin 0 -> 140 bytes .../96/cef4a7b72b3c208340ae6f0cf55a93e9077c93 | Bin 0 -> 163 bytes .../97/0c5deb117526983f554eaaa1b59102d3e3e0f7 | Bin 0 -> 32 bytes .../c5/626fc9eff57eb1bb7b796b01d4d0f2f3f792a2 | 2 + .../c7/04db5794097441aa2d9dd834d5b7e2f8f08108 | Bin 0 -> 163 bytes .../cb/ff181af4c9c7fee3cf6c106699e07d9a3f54e6 | Bin 0 -> 128 bytes .../d1/8e427f4011e74e96a31823c938be26eebab53b | Bin 0 -> 114 bytes .../d2/5795e38fbc1b4839697e834b957d61c83d994f | Bin 0 -> 32 bytes .../d6/6f456f0813a5841fbc03e5f1c47304dc675695 | Bin 0 -> 32 bytes .../e1/7e0fa20f3d2125916f2fb2f51f19240678cb83 | Bin 0 -> 219 bytes .../ec/d9fdda5c814055ee619513e1c388ba1bbcb280 | Bin 0 -> 32 bytes .../user2/commitsonpr.git/objects/info/packs | 1 + .../user2/commitsonpr.git/refs/heads/branch1 | 1 + .../user2/commitsonpr.git/refs/heads/main | 1 + .../user2/commitsonpr.git/refs/heads/master | 1 + .../user2/commitsonpr.git/refs/pull/1/head | 1 + tests/integration/api_issue_test.go | 14 +- tests/integration/api_nodeinfo_test.go | 2 +- tests/integration/api_repo_test.go | 6 +- tests/integration/issue_test.go | 14 +- tests/integration/pull_diff_test.go | 58 ++++ web_src/css/base.css | 5 + web_src/js/components/DiffCommitSelector.vue | 299 ++++++++++++++++++ web_src/js/features/repo-diff-commitselect.js | 10 + web_src/js/features/repo-diff.js | 2 + web_src/js/svg.js | 2 + 71 files changed, 748 insertions(+), 35 deletions(-) create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/HEAD create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/config create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/description create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/info/exclude create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/info/refs create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/logs/HEAD create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/logs/refs/heads/branch1 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/logs/refs/heads/main create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/0a/6dda431c72a6a4aac05b98e319972a1a55e01c create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/0c/396a509b64fd4e2e55649d100b86e8b96cc0e5 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/10/0ef49565829e7bd83057d2dab88f58b00db831 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/10/9ab1c0b84e088d7edcf018379518b49361f285 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/19/78192d98bb1b65e11c2cf37da854fbf94bffd6 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/1e/67d753ac1f9097eff26f9d33eb80182344b72c create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/23/576dd018294e476c06e569b6b0f170d0558705 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/28/16bffda09c0f23775ea4be279de004d28a3803 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/35/f03b5e176ee6d24c86b5cca7009a5b0ba2a026 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/38/cdad2e40c989aabab3f2d0a27faf0f7be617d5 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/3e/64625bd6eb5bcba69ac97de6c8f507402df861 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/4b/860706d3eec5858324d4ba00db0423ca4cbf50 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/4c/a8bcaf27e28504df7bf996819665986b01c847 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/52/84ca7f5757816e67c098224a8367aa2544222d create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/53/9a24812705f77484568e6ad7db84764c1903c8 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/70/8605e8984e7fb9be58818e0e6d9f21bcefd63e create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/74/7ddb3506a4fa04a7747808eb56ae16f9e933dc create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/7e/d1d42eda9110676d5c3a7721965d6ed1afe83c create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/81/1d46c7e518f4f180afb862c0db5cb8c80529ce create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/83/2d33e438d2b4a86fba81cb67b32d1d61a828cb create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/83/7d5c8125633d7d258f93b998e867eab0145520 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/87/cdc1333f5f117a92f3cef78ebe0301114b3610 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/92/70b08497106eaa65fce8aa91f37c4780f76909 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/96/cef4a7b72b3c208340ae6f0cf55a93e9077c93 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/97/0c5deb117526983f554eaaa1b59102d3e3e0f7 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/c5/626fc9eff57eb1bb7b796b01d4d0f2f3f792a2 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/c7/04db5794097441aa2d9dd834d5b7e2f8f08108 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/cb/ff181af4c9c7fee3cf6c106699e07d9a3f54e6 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/d1/8e427f4011e74e96a31823c938be26eebab53b create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/d2/5795e38fbc1b4839697e834b957d61c83d994f create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/d6/6f456f0813a5841fbc03e5f1c47304dc675695 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/e1/7e0fa20f3d2125916f2fb2f51f19240678cb83 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/ec/d9fdda5c814055ee619513e1c388ba1bbcb280 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/objects/info/packs create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/refs/heads/branch1 create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/refs/heads/main create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/refs/heads/master create mode 100644 tests/gitea-repositories-meta/user2/commitsonpr.git/refs/pull/1/head create mode 100644 tests/integration/pull_diff_test.go create mode 100644 web_src/js/components/DiffCommitSelector.vue create mode 100644 web_src/js/features/repo-diff-commitselect.js diff --git a/models/fixtures/issue.yml b/models/fixtures/issue.yml index 174345ff5a..fa72f9b647 100644 --- a/models/fixtures/issue.yml +++ b/models/fixtures/issue.yml @@ -304,3 +304,20 @@ created_unix: 946684830 updated_unix: 978307200 is_locked: false + +- + id: 19 + repo_id: 58 + index: 1 + poster_id: 2 + original_author_id: 0 + name: issue for pr + content: content + milestone_id: 0 + priority: 0 + is_closed: false + is_pull: true + num_comments: 0 + created_unix: 946684830 + updated_unix: 978307200 + is_locked: false diff --git a/models/fixtures/pull_request.yml b/models/fixtures/pull_request.yml index 165437f032..e5589ac703 100644 --- a/models/fixtures/pull_request.yml +++ b/models/fixtures/pull_request.yml @@ -76,3 +76,16 @@ base_branch: master merge_base: 2a47ca4b614a9f5a has_merged: false + +- + id: 7 + type: 0 # gitea pull request + status: 2 # mergable + issue_id: 19 + index: 1 + head_repo_id: 58 + base_repo_id: 58 + head_branch: branch1 + base_branch: main + merge_base: cbff181af4c9c7fee3cf6c106699e07d9a3f54e6 + has_merged: false diff --git a/models/fixtures/repo_unit.yml b/models/fixtures/repo_unit.yml index 5bb974a7d7..bb8715a202 100644 --- a/models/fixtures/repo_unit.yml +++ b/models/fixtures/repo_unit.yml @@ -607,3 +607,33 @@ repo_id: 52 type: 1 created_unix: 946684810 + +- + id: 91 + repo_id: 58 + type: 1 + created_unix: 946684810 + +- + id: 92 + repo_id: 58 + type: 2 + created_unix: 946684810 + +- + id: 93 + repo_id: 58 + type: 3 + created_unix: 946684810 + +- + id: 94 + repo_id: 58 + type: 4 + created_unix: 946684810 + +- + id: 95 + repo_id: 58 + type: 5 + created_unix: 946684810 diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml index 050a9e2d06..15668e6cae 100644 --- a/models/fixtures/repository.yml +++ b/models/fixtures/repository.yml @@ -1662,3 +1662,34 @@ is_private: false status: 0 num_issues: 0 + +- + id: 58 # org public repo + owner_id: 2 + owner_name: user2 + lower_name: commitsonpr + name: commitsonpr + default_branch: main + num_watches: 0 + num_stars: 0 + num_forks: 0 + num_issues: 0 + num_closed_issues: 0 + num_pulls: 1 + num_closed_pulls: 0 + num_milestones: 0 + num_closed_milestones: 0 + num_projects: 0 + num_closed_projects: 0 + is_private: false + is_empty: false + is_archived: false + is_mirror: false + status: 0 + is_fork: false + fork_id: 0 + is_template: false + template_id: 0 + size: 0 + is_fsck_enabled: true + close_issues_via_commit_in_any_branch: false diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index 26bb7a9f4b..c7c5c024be 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -66,7 +66,7 @@ num_followers: 2 num_following: 1 num_stars: 2 - num_repos: 13 + num_repos: 14 num_teams: 0 num_members: 0 visibility: 0 diff --git a/models/issues/issue_test.go b/models/issues/issue_test.go index 7f1eab1971..44e59f5cc4 100644 --- a/models/issues/issue_test.go +++ b/models/issues/issue_test.go @@ -538,7 +538,7 @@ func TestCountIssues(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) count, err := issues_model.CountIssues(db.DefaultContext, &issues_model.IssuesOptions{}) assert.NoError(t, err) - assert.EqualValues(t, 18, count) + assert.EqualValues(t, 19, count) } func TestIssueLoadAttributes(t *testing.T) { diff --git a/models/issues/review_list.go b/models/issues/review_list.go index c044fe915a..9f50d8e09d 100644 --- a/models/issues/review_list.go +++ b/models/issues/review_list.go @@ -114,7 +114,7 @@ func FindLatestReviews(ctx context.Context, opts FindReviewOptions) (ReviewList, } sess.In("id", builder. - Select("max ( id ) "). + Select("max(id)"). From("review"). Where(cond). GroupBy("reviewer_id")) diff --git a/models/repo/repo_list_test.go b/models/repo/repo_list_test.go index 573281ea0b..e013953c68 100644 --- a/models/repo/repo_list_test.go +++ b/models/repo/repo_list_test.go @@ -235,12 +235,12 @@ func TestSearchRepository(t *testing.T) { { name: "AllPublic/PublicRepositoriesOfUserIncludingCollaborative", opts: &repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, AllPublic: true, Template: util.OptionalBoolFalse}, - count: 30, + count: 31, }, { name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborative", opts: &repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, Private: true, AllPublic: true, AllLimited: true, Template: util.OptionalBoolFalse}, - count: 35, + count: 36, }, { name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborativeByName", @@ -255,7 +255,7 @@ func TestSearchRepository(t *testing.T) { { name: "AllPublic/PublicRepositoriesOfOrganization", opts: &repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 17, AllPublic: true, Collaborate: util.OptionalBoolFalse, Template: util.OptionalBoolFalse}, - count: 30, + count: 31, }, { name: "AllTemplates", diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index dc88c422b5..cf04830cbb 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1662,6 +1662,13 @@ pulls.switch_comparison_type = Switch comparison type pulls.switch_head_and_base = Switch head and base pulls.filter_branch = Filter branch pulls.no_results = No results found. +pulls.show_all_commits = Show all commits +pulls.show_changes_since_your_last_review = Show changes since your last review +pulls.showing_only_single_commit = Showing only changes of commit %[1]s +pulls.showing_specified_commit_range = Showing only changes between %[1]s..%[2]s +pulls.select_commit_hold_shift_for_range = Select commit. Hold shift + click to select a range +pulls.review_only_possible_for_full_diff = Review is only possible when viewing the full diff +pulls.filter_changes_by_commit = Filter by commit pulls.nothing_to_compare = These branches are equal. There is no need to create a pull request. pulls.nothing_to_compare_and_allow_empty_pr = These branches are equal. This PR will be empty. pulls.has_pull_request = `A pull request between these branches already exists: %[2]s#%[3]d` diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 237e53413f..a24ef43367 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -694,6 +694,42 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C return compareInfo } +type pullCommitList struct { + Commits []pull_service.CommitInfo `json:"commits"` + LastReviewCommitSha string `json:"last_review_commit_sha"` + Locale map[string]string `json:"locale"` +} + +// GetPullCommits get all commits for given pull request +func GetPullCommits(ctx *context.Context) { + issue := checkPullInfo(ctx) + if ctx.Written() { + return + } + resp := &pullCommitList{} + + commits, lastReviewCommitSha, err := pull_service.GetPullCommits(ctx, issue) + if err != nil { + ctx.JSON(http.StatusInternalServerError, err) + return + } + + // Get the needed locale + resp.Locale = map[string]string{ + "lang": ctx.Locale.Language(), + "filter_changes_by_commit": ctx.Tr("repo.pulls.filter_changes_by_commit"), + "show_all_commits": ctx.Tr("repo.pulls.show_all_commits"), + "stats_num_commits": ctx.TrN(len(commits), "repo.activity.git_stats_commit_1", "repo.activity.git_stats_commit_n", len(commits)), + "show_changes_since_your_last_review": ctx.Tr("repo.pulls.show_changes_since_your_last_review"), + "select_commit_hold_shift_for_range": ctx.Tr("repo.pulls.select_commit_hold_shift_for_range"), + } + + resp.Commits = commits + resp.LastReviewCommitSha = lastReviewCommitSha + + ctx.JSON(http.StatusOK, resp) +} + // ViewPullCommits show commits for a pull request func ViewPullCommits(ctx *context.Context) { ctx.Data["PageIsPullList"] = true @@ -739,7 +775,7 @@ func ViewPullCommits(ctx *context.Context) { } // ViewPullFiles render pull request changed files list page -func ViewPullFiles(ctx *context.Context) { +func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommit string, willShowSpecifiedCommitRange, willShowSpecifiedCommit bool) { ctx.Data["PageIsPullList"] = true ctx.Data["PageIsPullFiles"] = true @@ -762,6 +798,33 @@ func ViewPullFiles(ctx *context.Context) { prInfo = PrepareViewPullInfo(ctx, issue) } + // Validate the given commit sha to show (if any passed) + if willShowSpecifiedCommit || willShowSpecifiedCommitRange { + + foundStartCommit := len(specifiedStartCommit) == 0 + foundEndCommit := len(specifiedEndCommit) == 0 + + if !(foundStartCommit && foundEndCommit) { + for _, commit := range prInfo.Commits { + if commit.ID.String() == specifiedStartCommit { + foundStartCommit = true + } + if commit.ID.String() == specifiedEndCommit { + foundEndCommit = true + } + + if foundStartCommit && foundEndCommit { + break + } + } + } + + if !(foundStartCommit && foundEndCommit) { + ctx.NotFound("Given SHA1 not found for this PR", nil) + return + } + } + if ctx.Written() { return } else if prInfo == nil { @@ -775,12 +838,30 @@ func ViewPullFiles(ctx *context.Context) { return } - startCommitID = prInfo.MergeBase - endCommitID = headCommitID + ctx.Data["IsShowingOnlySingleCommit"] = willShowSpecifiedCommit + + if willShowSpecifiedCommit || willShowSpecifiedCommitRange { + if len(specifiedEndCommit) > 0 { + endCommitID = specifiedEndCommit + } else { + endCommitID = headCommitID + } + if len(specifiedStartCommit) > 0 { + startCommitID = specifiedStartCommit + } else { + startCommitID = prInfo.MergeBase + } + ctx.Data["IsShowingAllCommits"] = false + } else { + endCommitID = headCommitID + startCommitID = prInfo.MergeBase + ctx.Data["IsShowingAllCommits"] = true + } ctx.Data["Username"] = ctx.Repo.Owner.Name ctx.Data["Reponame"] = ctx.Repo.Repository.Name ctx.Data["AfterCommitID"] = endCommitID + ctx.Data["BeforeCommitID"] = startCommitID fileOnly := ctx.FormBool("file-only") @@ -789,8 +870,8 @@ func ViewPullFiles(ctx *context.Context) { if fileOnly && (len(files) == 2 || len(files) == 1) { maxLines, maxFiles = -1, -1 } + diffOptions := &gitdiff.DiffOptions{ - BeforeCommitID: startCommitID, AfterCommitID: endCommitID, SkipTo: ctx.FormString("skip-to"), MaxLines: maxLines, @@ -799,9 +880,18 @@ func ViewPullFiles(ctx *context.Context) { WhitespaceBehavior: gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)), } + if !willShowSpecifiedCommit { + diffOptions.BeforeCommitID = startCommitID + } + var methodWithError string var diff *gitdiff.Diff - if !ctx.IsSigned { + + // if we're not logged in or only a single commit (or commit range) is shown we + // have to load only the diff and not get the viewed information + // as the viewed information is designed to be loaded only on latest PR + // diff and if you're signed in. + if !ctx.IsSigned || willShowSpecifiedCommit || willShowSpecifiedCommitRange { diff, err = gitdiff.GetDiff(gitRepo, diffOptions, files...) methodWithError = "GetDiff" } else { @@ -908,6 +998,22 @@ func ViewPullFiles(ctx *context.Context) { ctx.HTML(http.StatusOK, tplPullFiles) } +func ViewPullFilesForSingleCommit(ctx *context.Context) { + viewPullFiles(ctx, "", ctx.Params("sha"), true, true) +} + +func ViewPullFilesForRange(ctx *context.Context) { + viewPullFiles(ctx, ctx.Params("shaFrom"), ctx.Params("shaTo"), true, false) +} + +func ViewPullFilesStartingFromCommit(ctx *context.Context) { + viewPullFiles(ctx, "", ctx.Params("sha"), true, false) +} + +func ViewPullFilesForAllCommitsOfPr(ctx *context.Context) { + viewPullFiles(ctx, "", "", false, false) +} + // UpdatePullRequest merge PR's baseBranch into headBranch func UpdatePullRequest(ctx *context.Context) { issue := checkPullInfo(ctx) diff --git a/routers/web/user/home_test.go b/routers/web/user/home_test.go index 3a06a38c24..634a91545e 100644 --- a/routers/web/user/home_test.go +++ b/routers/web/user/home_test.go @@ -75,7 +75,7 @@ func TestPulls(t *testing.T) { Pulls(ctx) assert.EqualValues(t, http.StatusOK, ctx.Resp.Status()) - assert.Len(t, ctx.Data["Issues"], 4) + assert.Len(t, ctx.Data["Issues"], 5) } func TestMilestones(t *testing.T) { diff --git a/routers/web/web.go b/routers/web/web.go index 0b51961445..ffc26dc291 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1279,14 +1279,20 @@ func registerRoutes(m *web.Route) { m.Get("", repo.SetWhitespaceBehavior, repo.GetPullDiffStats, repo.ViewIssue) m.Get(".diff", repo.DownloadPullDiff) m.Get(".patch", repo.DownloadPullPatch) - m.Get("/commits", context.RepoRef(), repo.SetWhitespaceBehavior, repo.GetPullDiffStats, repo.ViewPullCommits) + m.Group("/commits", func() { + m.Get("", context.RepoRef(), repo.SetWhitespaceBehavior, repo.GetPullDiffStats, repo.ViewPullCommits) + m.Get("/list", context.RepoRef(), repo.GetPullCommits) + m.Get("/{sha:[a-f0-9]{7,40}}", context.RepoRef(), repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesForSingleCommit) + }) m.Post("/merge", context.RepoMustNotBeArchived(), web.Bind(forms.MergePullRequestForm{}), repo.MergePullRequest) m.Post("/cancel_auto_merge", context.RepoMustNotBeArchived(), repo.CancelAutoMergePullRequest) m.Post("/update", repo.UpdatePullRequest) m.Post("/set_allow_maintainer_edit", web.Bind(forms.UpdateAllowEditsForm{}), repo.SetAllowEdits) m.Post("/cleanup", context.RepoMustNotBeArchived(), context.RepoRef(), repo.CleanUpPullRequest) m.Group("/files", func() { - m.Get("", context.RepoRef(), repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFiles) + m.Get("", context.RepoRef(), repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesForAllCommitsOfPr) + m.Get("/{sha:[a-f0-9]{7,40}}", context.RepoRef(), repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesStartingFromCommit) + m.Get("/{shaFrom:[a-f0-9]{7,40}}..{shaTo:[a-f0-9]{7,40}}", context.RepoRef(), repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesForRange) m.Group("/reviews", func() { m.Get("/new_comment", repo.RenderNewCodeCommentForm) m.Post("/comments", web.Bind(forms.CodeCommentForm{}), repo.SetShowOutdatedComments, repo.CreateCodeComment) diff --git a/services/pull/pull.go b/services/pull/pull.go index 3c6caec882..8c0b65fd8b 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -10,6 +10,7 @@ import ( "os" "regexp" "strings" + "time" "code.gitea.io/gitea/models" "code.gitea.io/gitea/models/db" @@ -17,7 +18,9 @@ import ( issues_model "code.gitea.io/gitea/models/issues" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/container" + gitea_context "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/json" @@ -856,3 +859,71 @@ func IsHeadEqualWithBranch(ctx context.Context, pr *issues_model.PullRequest, br } return baseCommit.HasPreviousCommit(headCommit.ID) } + +type CommitInfo struct { + Summary string `json:"summary"` + CommitterOrAuthorName string `json:"committer_or_author_name"` + ID string `json:"id"` + ShortSha string `json:"short_sha"` + Time string `json:"time"` +} + +// GetPullCommits returns all commits on given pull request and the last review commit sha +func GetPullCommits(ctx *gitea_context.Context, issue *issues_model.Issue) ([]CommitInfo, string, error) { + pull := issue.PullRequest + + baseGitRepo := ctx.Repo.GitRepo + + if err := pull.LoadBaseRepo(ctx); err != nil { + return nil, "", err + } + baseBranch := pull.BaseBranch + if pull.HasMerged { + baseBranch = pull.MergeBase + } + prInfo, err := baseGitRepo.GetCompareInfo(pull.BaseRepo.RepoPath(), baseBranch, pull.GetGitRefName(), true, false) + if err != nil { + return nil, "", err + } + + commits := make([]CommitInfo, 0, len(prInfo.Commits)) + + for _, commit := range prInfo.Commits { + var committerOrAuthorName string + var commitTime time.Time + if commit.Committer != nil { + committerOrAuthorName = commit.Committer.Name + commitTime = commit.Committer.When + } else { + committerOrAuthorName = commit.Author.Name + commitTime = commit.Author.When + } + + commits = append(commits, CommitInfo{ + Summary: commit.Summary(), + CommitterOrAuthorName: committerOrAuthorName, + ID: commit.ID.String(), + ShortSha: base.ShortSha(commit.ID.String()), + Time: commitTime.Format(time.RFC3339), + }) + } + + var lastReviewCommitID string + if ctx.IsSigned { + // get last review of current user and store information in context (if available) + lastreview, err := issues_model.FindLatestReviews(ctx, issues_model.FindReviewOptions{ + IssueID: issue.ID, + ReviewerID: ctx.Doer.ID, + Type: issues_model.ReviewTypeUnknown, + }) + + if err != nil && !issues_model.IsErrReviewNotExist(err) { + return nil, "", err + } + if len(lastreview) > 0 { + lastReviewCommitID = lastreview[0].CommitID + } + } + + return commits, lastReviewCommitID, nil +} diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index 72e6731254..c4dd1f658d 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -31,12 +31,32 @@ {{end}} {{template "repo/diff/whitespace_dropdown" .}} {{template "repo/diff/options_dropdown" .}} + {{if .PageIsPullFiles}} +
+ {{/* + the following will be replaced by vue component + but this avoids any loading artifacts till the vue component is initialized + */}} +
+ {{svg "octicon-git-commit"}} +
+
+ {{end}} {{if and .PageIsPullFiles $.SignedUserID (not .IsArchived)}} {{template "repo/diff/new_review" .}} {{end}} {{if not .DiffNotAvailable}} + {{if and .IsShowingOnlySingleCommit .PageIsPullFiles}} +
+
{{.locale.Tr "repo.pulls.showing_only_single_commit" (ShortSha .BeforeCommitID)}} - {{.locale.Tr "repo.pulls.show_all_commits"}}
+
+ {{else if and (not .IsShowingAllCommits) .PageIsPullFiles}} +
+
{{.locale.Tr "repo.pulls.showing_specified_commit_range" (ShortSha .BeforeCommitID) (ShortSha .AfterCommitID)}} - {{.locale.Tr "repo.pulls.show_all_commits"}}
+
+ {{end}} + diff --git a/web_src/js/features/repo-diff-commitselect.js b/web_src/js/features/repo-diff-commitselect.js new file mode 100644 index 0000000000..ebac64e855 --- /dev/null +++ b/web_src/js/features/repo-diff-commitselect.js @@ -0,0 +1,10 @@ +import {createApp} from 'vue'; +import DiffCommitSelector from '../components/DiffCommitSelector.vue'; + +export function initDiffCommitSelect() { + const el = document.getElementById('diff-commit-select'); + if (!el) return; + + const commitSelect = createApp(DiffCommitSelector); + commitSelect.mount(el); +} diff --git a/web_src/js/features/repo-diff.js b/web_src/js/features/repo-diff.js index f4bb724fe5..b79ca0f5b1 100644 --- a/web_src/js/features/repo-diff.js +++ b/web_src/js/features/repo-diff.js @@ -2,6 +2,7 @@ import $ from 'jquery'; import {initCompReactionSelector} from './comp/ReactionSelector.js'; import {initRepoIssueContentHistory} from './repo-issue-content.js'; import {initDiffFileTree} from './repo-diff-filetree.js'; +import {initDiffCommitSelect} from './repo-diff-commitselect.js'; import {validateTextareaNonEmpty} from './comp/ComboMarkdownEditor.js'; import {initViewedCheckboxListenerFor, countAndUpdateViewedFiles, initExpandAndCollapseFilesButton} from './pull-view-file.js'; import {initImageDiff} from './imagediff.js'; @@ -188,6 +189,7 @@ export function initRepoDiffView() { const diffFileList = $('#diff-file-list'); if (diffFileList.length === 0) return; initDiffFileTree(); + initDiffCommitSelect(); initRepoDiffShowMore(); initRepoDiffReviewButton(); initRepoDiffFileViewToggle(); diff --git a/web_src/js/svg.js b/web_src/js/svg.js index 2ef839aa21..46372e7d62 100644 --- a/web_src/js/svg.js +++ b/web_src/js/svg.js @@ -29,6 +29,7 @@ import octiconFileDirectoryFill from '../../public/assets/img/svg/octicon-file-d import octiconFilter from '../../public/assets/img/svg/octicon-filter.svg'; import octiconGear from '../../public/assets/img/svg/octicon-gear.svg'; import octiconGitBranch from '../../public/assets/img/svg/octicon-git-branch.svg'; +import octiconGitCommit from '../../public/assets/img/svg/octicon-git-commit.svg'; import octiconGitMerge from '../../public/assets/img/svg/octicon-git-merge.svg'; import octiconGitPullRequest from '../../public/assets/img/svg/octicon-git-pull-request.svg'; import octiconHeading from '../../public/assets/img/svg/octicon-heading.svg'; @@ -99,6 +100,7 @@ const svgs = { 'octicon-filter': octiconFilter, 'octicon-gear': octiconGear, 'octicon-git-branch': octiconGitBranch, + 'octicon-git-commit': octiconGitCommit, 'octicon-git-merge': octiconGitMerge, 'octicon-git-pull-request': octiconGitPullRequest, 'octicon-heading': octiconHeading,