mirror of
https://github.com/go-gitea/gitea
synced 2025-07-22 18:28:37 +00:00
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  ### Selecting a commit range  ### Show changes of a single commit only  ### Show changes of a commit range  Fixes https://github.com/go-gitea/gitea/issues/20989 Fixes https://github.com/go-gitea/gitea/issues/19263 --------- Co-authored-by: silverwind <me@silverwind.io> Co-authored-by: KN4CK3R <admin@oldschoolhack.me> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: delvh <dev.lh@web.de>
This commit is contained in:
@@ -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)
|
||||
|
@@ -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) {
|
||||
|
@@ -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)
|
||||
|
Reference in New Issue
Block a user