Refactor pull request view (2) (#37428)

Follow up #37380

Some code is moved to the place whether it should be.
This commit is contained in:
wxiaoguang
2026-04-26 21:58:48 +08:00
committed by GitHub
parent 712b3a54b5
commit b3ed4cde9a
4 changed files with 84 additions and 99 deletions

View File

@@ -908,8 +908,8 @@ func MarkConversation(ctx context.Context, comment *Comment, doer *user_model.Us
// CanMarkConversation Add or remove Conversation mark for a code comment permission check
// the PR writer , official reviewer and poster can do it
func CanMarkConversation(ctx context.Context, issue *Issue, doer *user_model.User) (permResult bool, err error) {
if doer == nil || issue == nil {
return false, errors.New("issue or doer is nil")
if doer == nil {
return false, nil
}
if err = issue.LoadRepo(ctx); err != nil {

View File

@@ -117,19 +117,6 @@ func roleDescriptor(ctx *context.Context, repo *repo_model.Repository, poster *u
return roleDesc, nil
}
func getBranchData(ctx *context.Context, issue *issues_model.Issue) {
ctx.Data["BaseBranch"] = nil
ctx.Data["HeadBranch"] = nil
ctx.Data["HeadUserName"] = nil
ctx.Data["BaseName"] = ctx.Repo.Repository.OwnerName
if issue.IsPull {
pull := issue.PullRequest
ctx.Data["BaseBranch"] = pull.BaseBranch
ctx.Data["HeadBranch"] = pull.HeadBranch
ctx.Data["HeadUserName"] = pull.MustHeadUserName(ctx)
}
}
// checkBlockedByIssues return canRead and notPermitted
func checkBlockedByIssues(ctx *context.Context, blockers []*issues_model.DependencyInfo) (canRead, notPermitted []*issues_model.DependencyInfo) {
repoPerms := make(map[int64]access_model.Permission)
@@ -379,6 +366,7 @@ func ViewIssue(ctx *context.Context) {
}
pageMetaData.LabelsData.SetSelectedLabels(issue.Labels)
prViewInfo := newPullRequestViewInfo()
prepareFuncs := []func(*context.Context, *issues_model.Issue){
prepareIssueViewContent,
prepareIssueViewCommentsAndSidebarParticipants,
@@ -389,8 +377,8 @@ func ViewIssue(ctx *context.Context) {
}
if issue.IsPull {
prepareFuncs = append(prepareFuncs,
func(ctx *context.Context, issue *issues_model.Issue) { preparePullViewPullInfo(ctx, issue) },
preparePullViewReviewAndMerge,
prViewInfo.prepareViewInfo,
prViewInfo.prepareMergeBox,
)
}
for _, prepareFunc := range prepareFuncs {
@@ -405,7 +393,7 @@ func ViewIssue(ctx *context.Context) {
if issue.PullRequest.HasMerged {
ctx.Data["DisableStatusChange"] = issue.PullRequest.HasMerged
} else {
ctx.Data["DisableStatusChange"] = ctx.Data["IsPullRequestBroken"] == true && issue.IsClosed
ctx.Data["DisableStatusChange"] = prViewInfo.IsPullRequestBroken && issue.IsClosed
}
}
@@ -445,11 +433,12 @@ func ViewPullMergeBox(ctx *context.Context) {
ctx.NotFound(nil)
return
}
preparePullViewPullInfo(ctx, issue)
prViewInfo := newPullRequestViewInfo()
prViewInfo.prepareViewInfo(ctx, issue)
if ctx.Written() {
return
}
preparePullViewReviewAndMerge(ctx, issue)
prViewInfo.prepareMergeBox(ctx, issue)
if ctx.Written() {
return
}
@@ -566,14 +555,11 @@ func prepareIssueViewSidebarTimeTracker(ctx *context.Context, issue *issues_mode
}
}
func preparePullViewDeleteBranch(ctx *context.Context, issue *issues_model.Issue, canDelete bool) {
if !issue.IsPull {
return
}
pull := issue.PullRequest
func (prInfo *pullRequestViewInfo) prepareMergeBoxDeleteBranch(ctx *context.Context, canDelete bool) {
pull := prInfo.issue.PullRequest
isPullBranchDeletable := canDelete &&
pull.HeadRepo != nil &&
(!pull.HasMerged || ctx.Data["HeadBranchCommitID"] == ctx.Data["PullHeadCommitID"])
(!pull.HasMerged || prInfo.HeadBranchCommitID == prInfo.CompareInfo.HeadCommitID)
if isPullBranchDeletable {
isPullBranchDeletable, _ = git_model.IsBranchExist(ctx, pull.HeadRepo.ID, pull.HeadBranch)
}
@@ -835,10 +821,9 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue
ctx.Data["NumParticipants"] = len(participants)
}
func preparePullViewReviewAndMerge(ctx *context.Context, issue *issues_model.Issue) {
getBranchData(ctx, issue)
if !issue.IsPull {
return
func (prInfo *pullRequestViewInfo) prepareMergeBox(ctx *context.Context, issue *issues_model.Issue) {
if prInfo.issue != issue {
panic("impossible, issue must be the same")
}
pull := issue.PullRequest
@@ -849,6 +834,22 @@ func preparePullViewReviewAndMerge(ctx *context.Context, issue *issues_model.Iss
pull_service.StartPullRequestCheckOnView(ctx, pull)
ctx.Data["GetCommitMessages"] = ""
if !prInfo.IsPullRequestBroken {
var err error
ctx.Data["UpdateAllowed"], ctx.Data["UpdateByRebaseAllowed"], err = pull_service.IsUserAllowedToUpdate(ctx, pull, ctx.Doer)
if err != nil {
ctx.ServerError("IsUserAllowedToUpdate", err)
return
}
ctx.Data["GetCommitMessages"] = pull_service.GetSquashMergeCommitMessages(ctx, pull)
}
if pull.IsFilesConflicted() {
ctx.Data["IsPullFilesConflicted"] = true
ctx.Data["ConflictedFiles"] = pull.ConflictedFiles
}
if ctx.IsSigned {
if err := pull.LoadHeadRepo(ctx); err != nil {
log.Error("LoadHeadRepo: %v", err)
@@ -974,7 +975,7 @@ func preparePullViewReviewAndMerge(ctx *context.Context, issue *issues_model.Iss
return
}
preparePullViewDeleteBranch(ctx, issue, canDelete)
prInfo.prepareMergeBoxDeleteBranch(ctx, canDelete)
if ctx.Written() {
return
}

View File

@@ -161,7 +161,8 @@ func getPullInfo(ctx *context.Context) (issue *issues_model.Issue, ok bool) {
return issue, true
}
func setMergeTarget(ctx *context.Context, pull *issues_model.PullRequest) {
func (prInfo *pullRequestViewInfo) setTemplateDataMergeTarget(ctx *context.Context) {
pull := prInfo.issue.PullRequest
if ctx.Repo.Owner.Name == pull.MustHeadUserName(ctx) {
ctx.Data["HeadTarget"] = pull.HeadBranch
} else if pull.HeadRepo == nil {
@@ -260,11 +261,13 @@ func GetMergedBaseCommitID(ctx *context.Context, issue *issues_model.Issue) stri
return baseCommit
}
// PullRequestViewInfo is a structured type for viewing pull request
// pullRequestViewInfo is a structured type for viewing pull request
// Refactoring plan:
// * move dynamic template-data-based variable into this struct
// * let backend handle complex logic, prepare everything, avoid plenty of "if" blocks in tmpl
type PullRequestViewInfo struct {
type pullRequestViewInfo struct {
issue *issues_model.Issue
IsPullRequestBroken bool
HeadBranchCommitID string
@@ -277,41 +280,47 @@ type PullRequestViewInfo struct {
CommitStatuses []*git_model.CommitStatus
}
func preparePullViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *PullRequestViewInfo {
prInfo := &PullRequestViewInfo{}
ctx.Data["PullRequestViewInfo"] = prInfo // TODO: after the complete refactoring, decouple the variable from template data
func newPullRequestViewInfo() *pullRequestViewInfo {
return &pullRequestViewInfo{}
}
func (prInfo *pullRequestViewInfo) prepareViewInfo(ctx *context.Context, issue *issues_model.Issue) {
prInfo.issue = issue
ctx.Data["PullRequestWorkInProgressPrefixes"] = setting.Repository.PullRequest.WorkInProgressPrefixes
if err := issue.PullRequest.LoadHeadRepo(ctx); err != nil {
ctx.ServerError("LoadHeadRepo", err)
return nil
return
}
if err := issue.PullRequest.LoadBaseRepo(ctx); err != nil {
ctx.ServerError("LoadBaseRepo", err)
return nil
return
}
// for the PR target branch selector
ctx.Data["BaseBranch"] = issue.PullRequest.BaseBranch
ctx.Data["HeadBranch"] = issue.PullRequest.HeadBranch
ctx.Data["HeadUserName"] = issue.PullRequest.MustHeadUserName(ctx)
if issue.PullRequest.HasMerged {
prepareViewMergedPullInfo(ctx, issue)
prInfo.prepareViewMergedPullInfo(ctx)
} else {
prepareViewOpenPullInfo(ctx, issue)
prInfo.prepareViewOpenPullInfo(ctx)
}
return prInfo
}
func preparePullViewFillInfo(ctx *context.Context, issue *issues_model.Issue, baseRef git.RefName) {
preparePullViewFillCompareInfo(ctx, issue, baseRef)
func (prInfo *pullRequestViewInfo) prepareViewFillInfo(ctx *context.Context, baseRef git.RefName) {
prInfo.prepareViewFillCompareInfo(ctx, baseRef)
if ctx.Written() {
return
}
preparePullViewFillCommitStatusInfo(ctx, issue)
prInfo.prepareViewFillCommitStatusInfo(ctx)
}
func preparePullViewFillCompareInfo(ctx *context.Context, issue *issues_model.Issue, baseRef git.RefName) {
func (prInfo *pullRequestViewInfo) prepareViewFillCompareInfo(ctx *context.Context, baseRef git.RefName) {
var err error
prInfo := ctx.Data["PullRequestViewInfo"].(*PullRequestViewInfo)
pull := issue.PullRequest
pull := prInfo.issue.PullRequest
prInfo.CompareInfo, err = git_service.GetCompareInfo(ctx, ctx.Repo.Repository, ctx.Repo.Repository, ctx.Repo.GitRepo, baseRef, git.RefName(pull.GetGitHeadRefName()), false, false)
if err != nil {
isKnownErrorForBroken := gitcmd.IsStdErrorNotValidObjectName(err) ||
@@ -339,12 +348,10 @@ func preparePullViewFillCompareInfo(ctx *context.Context, issue *issues_model.Is
ctx.Data["IsPullRequestBroken"] = prInfo.IsPullRequestBroken
ctx.Data["NumCommits"] = len(prInfo.CompareInfo.Commits)
ctx.Data["NumFiles"] = prInfo.CompareInfo.NumFiles
setMergeTarget(ctx, issue.PullRequest)
prInfo.setTemplateDataMergeTarget(ctx)
}
func preparePullViewFillCommitStatusInfo(ctx *context.Context, issue *issues_model.Issue) {
prInfo := ctx.Data["PullRequestViewInfo"].(*PullRequestViewInfo)
func (prInfo *pullRequestViewInfo) prepareViewFillCommitStatusInfo(ctx *context.Context) {
headCommitID := prInfo.CompareInfo.HeadCommitID
if headCommitID == "" {
return
@@ -369,13 +376,13 @@ func preparePullViewFillCommitStatusInfo(ctx *context.Context, issue *issues_mod
ctx.Data["LatestCommitStatus"] = statusCheckData.LatestCommitStatus
ctx.Data["StatusCheckData"] = &prInfo.StatusCheckData
if !issue.IsClosed {
preparePullViewFillCommitStatusInfoForOpen(ctx, issue)
if !prInfo.issue.IsClosed {
prInfo.prepareViewFillCommitStatusInfoForOpen(ctx)
}
}
func preparePullViewFillCommitStatusInfoForOpen(ctx *context.Context, issue *issues_model.Issue) {
prInfo := ctx.Data["PullRequestViewInfo"].(*PullRequestViewInfo)
func (prInfo *pullRequestViewInfo) prepareViewFillCommitStatusInfoForOpen(ctx *context.Context) {
issue := prInfo.issue
statusCheckData := &prInfo.StatusCheckData
commitStatuses := prInfo.CommitStatuses
runs, err := actions_service.GetRunsFromCommitStatuses(ctx, commitStatuses)
@@ -439,10 +446,10 @@ func preparePullViewFillCommitStatusInfoForOpen(ctx *context.Context, issue *iss
}
// prepareViewMergedPullInfo show meta information for a merged pull request view page
func prepareViewMergedPullInfo(ctx *context.Context, issue *issues_model.Issue) {
func (prInfo *pullRequestViewInfo) prepareViewMergedPullInfo(ctx *context.Context) {
ctx.Data["HasMerged"] = true
baseCommit := GetMergedBaseCommitID(ctx, issue)
preparePullViewFillInfo(ctx, issue, git.RefName(baseCommit))
baseCommit := GetMergedBaseCommitID(ctx, prInfo.issue)
prInfo.prepareViewFillInfo(ctx, git.RefName(baseCommit))
}
type pullCommitStatusCheckData struct {
@@ -495,49 +502,31 @@ func getViewPullHeadBranchCommitID(ctx *context.Context, pull *issues_model.Pull
return "", util.ErrNotExist
}
func prepareViewOpenPullInfo(ctx *context.Context, issue *issues_model.Issue) {
prInfo := ctx.Data["PullRequestViewInfo"].(*PullRequestViewInfo)
pull := issue.PullRequest
func (prInfo *pullRequestViewInfo) prepareViewOpenPullInfo(ctx *context.Context) {
pull := prInfo.issue.PullRequest
if exist, _ := git_model.IsBranchExist(ctx, pull.BaseRepo.ID, pull.BaseBranch); !exist {
// if base branch doesn't exist, prepare from the merge base
ctx.Data["BaseBranchNotExist"] = true
preparePullViewFillInfo(ctx, issue, git.RefName(pull.MergeBase))
prInfo.prepareViewFillInfo(ctx, git.RefName(pull.MergeBase))
return
}
preparePullViewFillInfo(ctx, issue, git.RefNameFromBranch(pull.BaseBranch))
prInfo.prepareViewFillInfo(ctx, git.RefNameFromBranch(pull.BaseBranch))
if ctx.Written() {
return
}
if !prInfo.IsPullRequestBroken {
var err error
ctx.Data["UpdateAllowed"], ctx.Data["UpdateByRebaseAllowed"], err = pull_service.IsUserAllowedToUpdate(ctx, pull, ctx.Doer)
if err != nil {
ctx.ServerError("IsUserAllowedToUpdate", err)
return
}
ctx.Data["GetCommitMessages"] = pull_service.GetSquashMergeCommitMessages(ctx, pull)
} else {
ctx.Data["GetCommitMessages"] = ""
}
ctx.Data["HeadBranchCommitID"] = prInfo.HeadBranchCommitID
ctx.Data["PullHeadCommitID"] = prInfo.CompareInfo.HeadCommitID
if prInfo.CompareInfo.HeadCommitID == prInfo.CompareInfo.MergeBase {
ctx.Data["IsNothingToCompare"] = true
}
// this one is used by both sidebar and merge-box
if pull.IsWorkInProgress(ctx) {
ctx.Data["IsPullWorkInProgress"] = true
ctx.Data["WorkInProgressPrefix"] = pull.GetWorkInProgressPrefix(ctx)
}
if pull.IsFilesConflicted() {
ctx.Data["IsPullFilesConflicted"] = true
ctx.Data["ConflictedFiles"] = pull.ConflictedFiles
}
}
func createRequiredContextMatcher(requiredContext string) func(string) bool {
@@ -596,8 +585,8 @@ func ViewPullCommits(ctx *context.Context) {
if !ok {
return
}
prViewInfo := preparePullViewPullInfo(ctx, issue)
prViewInfo := newPullRequestViewInfo()
prViewInfo.prepareViewInfo(ctx, issue)
if ctx.Written() {
return
}
@@ -626,7 +615,6 @@ func ViewPullCommits(ctx *context.Context) {
if ctx.Written() {
return
}
getBranchData(ctx, issue)
ctx.HTML(http.StatusOK, tplPullCommits)
}
@@ -652,7 +640,8 @@ func viewPullFiles(ctx *context.Context, beforeCommitID, afterCommitID string) {
gitRepo := ctx.Repo.GitRepo
prViewInfo := preparePullViewPullInfo(ctx, issue)
prViewInfo := newPullRequestViewInfo()
prViewInfo.prepareViewInfo(ctx, issue)
if ctx.Written() {
return
}
@@ -704,8 +693,6 @@ func viewPullFiles(ctx *context.Context, beforeCommitID, afterCommitID string) {
beforeCommitID = beforeCommit.ID.String()
}
ctx.Data["Username"] = ctx.Repo.Owner.Name
ctx.Data["Reponame"] = ctx.Repo.Repository.Name
ctx.Data["MergeBase"] = prCompareInfo.MergeBase
ctx.Data["AfterCommitID"] = afterCommitID
ctx.Data["BeforeCommitID"] = beforeCommitID
@@ -788,10 +775,10 @@ func viewPullFiles(ctx *context.Context, beforeCommitID, afterCommitID string) {
}
if pb != nil {
glob := pb.GetProtectedFilePatterns()
if len(glob) != 0 {
protectedFilePatterns := pb.GetProtectedFilePatterns()
if len(protectedFilePatterns) != 0 {
for _, file := range diff.Files {
file.IsProtected = pb.IsProtectedFile(glob, file.Name)
file.IsProtected = pb.IsProtectedFile(protectedFilePatterns, file.Name)
}
}
}
@@ -824,11 +811,9 @@ func viewPullFiles(ctx *context.Context, beforeCommitID, afterCommitID string) {
}
ctx.Data["DiffNotAvailable"] = diffShortStat.NumFiles == 0
if ctx.IsSigned && ctx.Doer != nil {
if ctx.Data["CanMarkConversation"], err = issues_model.CanMarkConversation(ctx, issue, ctx.Doer); err != nil {
ctx.ServerError("CanMarkConversation", err)
return
}
if ctx.Data["CanMarkConversation"], err = issues_model.CanMarkConversation(ctx, issue, ctx.Doer); err != nil {
ctx.ServerError("CanMarkConversation", err)
return
}
setCompareContext(ctx, beforeCommit, afterCommit, ctx.Repo.Owner.Name, ctx.Repo.Repository.Name)
@@ -860,8 +845,7 @@ func viewPullFiles(ctx *context.Context, beforeCommitID, afterCommitID string) {
ctx.Data["CurrentReview"] = currentReview
ctx.Data["PendingCodeCommentNumber"] = numPendingCodeComments
getBranchData(ctx, issue)
ctx.Data["IsIssuePoster"] = ctx.IsSigned && issue.IsPoster(ctx.Doer.ID)
ctx.Data["IsIssuePoster"] = ctx.Doer != nil && issue.IsPoster(ctx.Doer.ID)
ctx.Data["HasIssuesOrPullsWritePermission"] = ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull)
ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled

View File

@@ -30,7 +30,7 @@
{{$commitBaseLink = printf "%s/wiki/commit" $commitRepoLink}}
{{else if $.PageIsPullCommits}}
{{$commitBaseLink = printf "%s/pulls/%d/commits" $commitRepoLink $.Issue.Index}}
{{else if $.Reponame}}
{{else}}
{{$commitBaseLink = printf "%s/commit" $commitRepoLink}}
{{end}}
{{template "repo/commit_sign_badge" dict "Commit" . "CommitBaseLink" $commitBaseLink "CommitSignVerification" .Verification}}