From 699bd93d6874a24f5f80249fb9a0fc2494aa13d9 Mon Sep 17 00:00:00 2001 From: Lunny Xiao <xiaolunwen@gmail.com> Date: Fri, 6 Dec 2024 21:16:26 -0800 Subject: [PATCH 1/5] Split issue/pull view router function as multiple smaller functions --- models/issues/issue.go | 1 + routers/web/repo/issue_view.go | 423 +++++++++++++++++++-------------- 2 files changed, 245 insertions(+), 179 deletions(-) diff --git a/models/issues/issue.go b/models/issues/issue.go index 64fc20cc05e99..8038bb9717da1 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -302,6 +302,7 @@ func (issue *Issue) loadReactions(ctx context.Context) (err error) { comment.Reactions = append(comment.Reactions, react) } } + issue.Reactions = reactions return nil } diff --git a/routers/web/repo/issue_view.go b/routers/web/repo/issue_view.go index 54ff36db4925b..d035a6dd5b66c 100644 --- a/routers/web/repo/issue_view.go +++ b/routers/web/repo/issue_view.go @@ -325,14 +325,6 @@ func ViewIssue(ctx *context.Context) { ctx.Data["NewIssueChooseTemplate"] = issue_service.HasTemplatesOrContactLinks(ctx.Repo.Repository, ctx.Repo.GitRepo) } - if issue.IsPull && !ctx.Repo.CanRead(unit.TypeIssues) { - ctx.Data["IssueDependencySearchType"] = "pulls" - } else if !issue.IsPull && !ctx.Repo.CanRead(unit.TypePullRequests) { - ctx.Data["IssueDependencySearchType"] = "issues" - } else { - ctx.Data["IssueDependencySearchType"] = "all" - } - ctx.Data["IsProjectsEnabled"] = ctx.Repo.CanRead(unit.TypeProjects) ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled upload.AddUploadContext(ctx, "comment") @@ -349,26 +341,11 @@ func ViewIssue(ctx *context.Context) { ctx.Data["Title"] = fmt.Sprintf("#%d - %s", issue.Index, emoji.ReplaceAliases(issue.Title)) - iw := new(issues_model.IssueWatch) - if ctx.Doer != nil { - iw.UserID = ctx.Doer.ID - iw.IssueID = issue.ID - iw.IsWatching, err = issues_model.CheckIssueWatch(ctx, ctx.Doer, issue) - if err != nil { - ctx.ServerError("CheckIssueWatch", err) - return - } - } - ctx.Data["IssueWatch"] = iw - rctx := renderhelper.NewRenderContextRepoComment(ctx, ctx.Repo.Repository) - issue.RenderedContent, err = markdown.RenderString(rctx, issue.Content) - if err != nil { - ctx.ServerError("RenderString", err) + repo := ctx.Repo.Repository + if prepareIssueViewContent(ctx, issue) { return } - repo := ctx.Repo.Repository - // Get more information if it's a pull request. if issue.IsPull { if issue.PullRequest.HasMerged { @@ -397,24 +374,133 @@ func ViewIssue(ctx *context.Context) { } } - var ( - role issues_model.RoleDescriptor - ok bool - marked = make(map[int64]issues_model.RoleDescriptor) - comment *issues_model.Comment - participants = make([]*user_model.User, 1, 10) - latestCloseCommentID int64 - ) + if prepareIssueViewCommentsAndSidebarParticipants(ctx, issue) { + return + } + + if preparePullViewReviewAndMerge(ctx, issue) { + return + } + + if prepareIssueViewSidebarWatch(ctx, issue) { + return + } + if prepareIssueViewSidebarTimeTracker(ctx, issue) { + return + } + if prepareIssueViewSidebarDependency(ctx, issue) { + return + } + if prepareIssueViewSidebarPin(ctx, issue) { + return + } + + ctx.Data["Reference"] = issue.Ref + ctx.Data["SignInLink"] = setting.AppSubURL + "/user/login?redirect_to=" + url.QueryEscape(ctx.Data["Link"].(string)) + ctx.Data["IsIssuePoster"] = ctx.IsSigned && issue.IsPoster(ctx.Doer.ID) + ctx.Data["HasIssuesOrPullsWritePermission"] = ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) + ctx.Data["HasProjectsWritePermission"] = ctx.Repo.CanWrite(unit.TypeProjects) + ctx.Data["IsRepoAdmin"] = ctx.IsSigned && (ctx.Repo.IsAdmin() || ctx.Doer.IsAdmin) + ctx.Data["LockReasons"] = setting.Repository.Issue.LockReasons + ctx.Data["RefEndName"] = git.RefName(issue.Ref).ShortName() + + tags, err := repo_model.GetTagNamesByRepoID(ctx, ctx.Repo.Repository.ID) + if err != nil { + ctx.ServerError("GetTagNamesByRepoID", err) + return + } + ctx.Data["Tags"] = tags + + ctx.Data["CanBlockUser"] = func(blocker, blockee *user_model.User) bool { + return user_service.CanBlockUser(ctx, ctx.Doer, blocker, blockee) + } + + ctx.HTML(http.StatusOK, tplIssueView) +} + +func prepareIssueViewSidebarDependency(ctx *context.Context, issue *issues_model.Issue) bool { + if issue.IsPull && !ctx.Repo.CanRead(unit.TypeIssues) { + ctx.Data["IssueDependencySearchType"] = "pulls" + } else if !issue.IsPull && !ctx.Repo.CanRead(unit.TypePullRequests) { + ctx.Data["IssueDependencySearchType"] = "issues" + } else { + ctx.Data["IssueDependencySearchType"] = "all" + } + + // Check if the user can use the dependencies + ctx.Data["CanCreateIssueDependencies"] = ctx.Repo.CanCreateIssueDependencies(ctx, ctx.Doer, issue.IsPull) + + // check if dependencies can be created across repositories + ctx.Data["AllowCrossRepositoryDependencies"] = setting.Service.AllowCrossRepositoryDependencies + + // Get Dependencies + blockedBy, err := issue.BlockedByDependencies(ctx, db.ListOptions{}) + if err != nil { + ctx.ServerError("BlockedByDependencies", err) + return true + } + ctx.Data["BlockedByDependencies"], ctx.Data["BlockedByDependenciesNotPermitted"] = checkBlockedByIssues(ctx, blockedBy) + if ctx.Written() { + return true + } + + blocking, err := issue.BlockingDependencies(ctx) + if err != nil { + ctx.ServerError("BlockingDependencies", err) + return true + } + + ctx.Data["BlockingDependencies"], ctx.Data["BlockingDependenciesNotPermitted"] = checkBlockedByIssues(ctx, blocking) + return ctx.Written() +} + +func preparePullViewSigning(ctx *context.Context, issue *issues_model.Issue) bool { + pull := issue.PullRequest + ctx.Data["WillSign"] = false + if ctx.Doer != nil { + sign, key, _, err := asymkey_service.SignMerge(ctx, pull, ctx.Doer, pull.BaseRepo.RepoPath(), pull.BaseBranch, pull.GetGitRefName()) + ctx.Data["WillSign"] = sign + ctx.Data["SigningKey"] = key + if err != nil { + if asymkey_service.IsErrWontSign(err) { + ctx.Data["WontSignReason"] = err.(*asymkey_service.ErrWontSign).Reason + } else { + ctx.Data["WontSignReason"] = "error" + log.Error("Error whilst checking if could sign pr %d in repo %s. Error: %v", pull.ID, pull.BaseRepo.FullName(), err) + } + } + } else { + ctx.Data["WontSignReason"] = "not_signed_in" + } + return false +} + +func prepareIssueViewSidebarWatch(ctx *context.Context, issue *issues_model.Issue) bool { + iw := new(issues_model.IssueWatch) + if ctx.Doer != nil { + iw.UserID = ctx.Doer.ID + iw.IssueID = issue.ID + var err error + iw.IsWatching, err = issues_model.CheckIssueWatch(ctx, ctx.Doer, issue) + if err != nil { + ctx.ServerError("CheckIssueWatch", err) + return true + } + } + ctx.Data["IssueWatch"] = iw + return false +} + +func prepareIssueViewSidebarTimeTracker(ctx *context.Context, issue *issues_model.Issue) bool { if ctx.Repo.Repository.IsTimetrackerEnabled(ctx) { if ctx.IsSigned { // Deal with the stopwatch ctx.Data["IsStopwatchRunning"] = issues_model.StopwatchExists(ctx, ctx.Doer.ID, issue.ID) if !ctx.Data["IsStopwatchRunning"].(bool) { - var exists bool - var swIssue *issues_model.Issue - if exists, _, swIssue, err = issues_model.HasUserStopwatch(ctx, ctx.Doer.ID); err != nil { + exists, _, swIssue, err := issues_model.HasUserStopwatch(ctx, ctx.Doer.ID) + if err != nil { ctx.ServerError("HasUserStopwatch", err) - return + return true } ctx.Data["HasUserStopwatch"] = exists if exists { @@ -427,22 +513,67 @@ func ViewIssue(ctx *context.Context) { } else { ctx.Data["CanUseTimetracker"] = false } + var err error if ctx.Data["WorkingUsers"], err = issues_model.TotalTimesForEachUser(ctx, &issues_model.FindTrackedTimesOptions{IssueID: issue.ID}); err != nil { ctx.ServerError("TotalTimesForEachUser", err) - return + return true } } + return false +} - // Check if the user can use the dependencies - ctx.Data["CanCreateIssueDependencies"] = ctx.Repo.CanCreateIssueDependencies(ctx, ctx.Doer, issue.IsPull) +func preparePullViewDeleteBranch(ctx *context.Context, issue *issues_model.Issue, canDelete bool) bool { + if !issue.IsPull { + return false + } + pull := issue.PullRequest + isPullBranchDeletable := canDelete && + pull.HeadRepo != nil && + git.IsBranchExist(ctx, pull.HeadRepo.RepoPath(), pull.HeadBranch) && + (!pull.HasMerged || ctx.Data["HeadBranchCommitID"] == ctx.Data["PullHeadCommitID"]) - // check if dependencies can be created across repositories - ctx.Data["AllowCrossRepositoryDependencies"] = setting.Service.AllowCrossRepositoryDependencies + if isPullBranchDeletable && pull.HasMerged { + exist, err := issues_model.HasUnmergedPullRequestsByHeadInfo(ctx, pull.HeadRepoID, pull.HeadBranch) + if err != nil { + ctx.ServerError("HasUnmergedPullRequestsByHeadInfo", err) + return true + } - if issue.ShowRole, err = roleDescriptor(ctx, repo, issue.Poster, issue, issue.HasOriginalAuthor()); err != nil { - ctx.ServerError("roleDescriptor", err) - return + isPullBranchDeletable = !exist } + ctx.Data["IsPullBranchDeletable"] = isPullBranchDeletable + return false +} + +func prepareIssueViewSidebarPin(ctx *context.Context, issue *issues_model.Issue) bool { + var pinAllowed bool + if !issue.IsPinned() { + var err error + pinAllowed, err = issues_model.IsNewPinAllowed(ctx, issue.RepoID, issue.IsPull) + if err != nil { + ctx.ServerError("IsNewPinAllowed", err) + return true + } + } else { + pinAllowed = true + } + + ctx.Data["NewPinAllowed"] = pinAllowed + ctx.Data["PinEnabled"] = setting.Repository.Issue.MaxPinned != 0 + return false +} + +func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue *issues_model.Issue) bool { + var ( + role issues_model.RoleDescriptor + ok bool + marked = make(map[int64]issues_model.RoleDescriptor) + comment *issues_model.Comment + participants = make([]*user_model.User, 1, 10) + latestCloseCommentID int64 + err error + ) + marked[issue.PosterID] = issue.ShowRole // Render comments and fetch participants. @@ -450,22 +581,22 @@ func ViewIssue(ctx *context.Context) { if err := issue.Comments.LoadAttachmentsByIssue(ctx); err != nil { ctx.ServerError("LoadAttachmentsByIssue", err) - return + return true } if err := issue.Comments.LoadPosters(ctx); err != nil { ctx.ServerError("LoadPosters", err) - return + return true } for _, comment = range issue.Comments { comment.Issue = issue if comment.Type == issues_model.CommentTypeComment || comment.Type == issues_model.CommentTypeReview { - rctx = renderhelper.NewRenderContextRepoComment(ctx, repo) + rctx := renderhelper.NewRenderContextRepoComment(ctx, issue.Repo) comment.RenderedContent, err = markdown.RenderString(rctx, comment.Content) if err != nil { ctx.ServerError("RenderString", err) - return + return true } // Check tag. role, ok = marked[comment.PosterID] @@ -474,22 +605,22 @@ func ViewIssue(ctx *context.Context) { continue } - comment.ShowRole, err = roleDescriptor(ctx, repo, comment.Poster, issue, comment.HasOriginalAuthor()) + comment.ShowRole, err = roleDescriptor(ctx, issue.Repo, comment.Poster, issue, comment.HasOriginalAuthor()) if err != nil { ctx.ServerError("roleDescriptor", err) - return + return true } marked[comment.PosterID] = comment.ShowRole participants = addParticipant(comment.Poster, participants) } else if comment.Type == issues_model.CommentTypeLabel { if err = comment.LoadLabel(ctx); err != nil { ctx.ServerError("LoadLabel", err) - return + return true } } else if comment.Type == issues_model.CommentTypeMilestone { if err = comment.LoadMilestone(ctx); err != nil { ctx.ServerError("LoadMilestone", err) - return + return true } ghostMilestone := &issues_model.Milestone{ ID: -1, @@ -504,7 +635,7 @@ func ViewIssue(ctx *context.Context) { } else if comment.Type == issues_model.CommentTypeProject { if err = comment.LoadProject(ctx); err != nil { ctx.ServerError("LoadProject", err) - return + return true } ghostProject := &project_model.Project{ @@ -522,30 +653,30 @@ func ViewIssue(ctx *context.Context) { } else if comment.Type == issues_model.CommentTypeProjectColumn { if err = comment.LoadProject(ctx); err != nil { ctx.ServerError("LoadProject", err) - return + return true } } else if comment.Type == issues_model.CommentTypeAssignees || comment.Type == issues_model.CommentTypeReviewRequest { if err = comment.LoadAssigneeUserAndTeam(ctx); err != nil { ctx.ServerError("LoadAssigneeUserAndTeam", err) - return + return true } } else if comment.Type == issues_model.CommentTypeRemoveDependency || comment.Type == issues_model.CommentTypeAddDependency { if err = comment.LoadDepIssueDetails(ctx); err != nil { if !issues_model.IsErrIssueNotExist(err) { ctx.ServerError("LoadDepIssueDetails", err) - return + return true } } } else if comment.Type.HasContentSupport() { - rctx = renderhelper.NewRenderContextRepoComment(ctx, repo) + rctx := renderhelper.NewRenderContextRepoComment(ctx, issue.Repo) comment.RenderedContent, err = markdown.RenderString(rctx, comment.Content) if err != nil { ctx.ServerError("RenderString", err) - return + return true } if err = comment.LoadReview(ctx); err != nil && !issues_model.IsErrReviewNotExist(err) { ctx.ServerError("LoadReview", err) - return + return true } participants = addParticipant(comment.Poster, participants) if comment.Review == nil { @@ -554,13 +685,13 @@ func ViewIssue(ctx *context.Context) { if err = comment.Review.LoadAttributes(ctx); err != nil { if !user_model.IsErrUserNotExist(err) { ctx.ServerError("Review.LoadAttributes", err) - return + return true } comment.Review.Reviewer = user_model.NewGhostUser() } if err = comment.Review.LoadCodeComments(ctx); err != nil { ctx.ServerError("Review.LoadCodeComments", err) - return + return true } for _, codeComments := range comment.Review.CodeComments { for _, lineComments := range codeComments { @@ -572,10 +703,10 @@ func ViewIssue(ctx *context.Context) { continue } - c.ShowRole, err = roleDescriptor(ctx, repo, c.Poster, issue, c.HasOriginalAuthor()) + c.ShowRole, err = roleDescriptor(ctx, issue.Repo, c.Poster, issue, c.HasOriginalAuthor()) if err != nil { ctx.ServerError("roleDescriptor", err) - return + return true } marked[c.PosterID] = c.ShowRole participants = addParticipant(c.Poster, participants) @@ -584,13 +715,13 @@ func ViewIssue(ctx *context.Context) { } if err = comment.LoadResolveDoer(ctx); err != nil { ctx.ServerError("LoadResolveDoer", err) - return + return true } } else if comment.Type == issues_model.CommentTypePullRequestPush { participants = addParticipant(comment.Poster, participants) if err = comment.LoadPushCommits(ctx); err != nil { ctx.ServerError("LoadPushCommits", err) - return + return true } if !ctx.Repo.CanRead(unit.TypeActions) { for _, commit := range comment.Commits { @@ -629,6 +760,26 @@ func ViewIssue(ctx *context.Context) { // Combine multiple label assignments into a single comment combineLabelComments(issue) + var hiddenCommentTypes *big.Int + if ctx.IsSigned { + val, err := user_model.GetUserSetting(ctx, ctx.Doer.ID, user_model.SettingsKeyHiddenCommentTypes) + if err != nil { + ctx.ServerError("GetUserSetting", err) + return true + } + hiddenCommentTypes, _ = new(big.Int).SetString(val, 10) // we can safely ignore the failed conversion here + } + ctx.Data["ShouldShowCommentType"] = func(commentType issues_model.CommentType) bool { + return hiddenCommentTypes == nil || hiddenCommentTypes.Bit(int(commentType)) == 0 + } + + // prepare for sidebar participants + ctx.Data["Participants"] = participants + ctx.Data["NumParticipants"] = len(participants) + return false +} + +func preparePullViewReviewAndMerge(ctx *context.Context, issue *issues_model.Issue) bool { getBranchData(ctx, issue) if issue.IsPull { pull := issue.PullRequest @@ -644,7 +795,7 @@ func ViewIssue(ctx *context.Context) { perm, err := access_model.GetUserRepoPermission(ctx, pull.HeadRepo, ctx.Doer) if err != nil { ctx.ServerError("GetUserRepoPermission", err) - return + return true } if perm.CanWrite(unit.TypeCode) { // Check if branch is not protected @@ -666,7 +817,7 @@ func ViewIssue(ctx *context.Context) { perm, err := access_model.GetUserRepoPermission(ctx, pull.BaseRepo, ctx.Doer) if err != nil { ctx.ServerError("GetUserRepoPermission", err) - return + return true } if !canWriteToHeadRepo { // maintainers maybe allowed to push to head repo even if they can't write to it canWriteToHeadRepo = pull.AllowMaintainerEdit && perm.CanWrite(unit.TypeCode) @@ -674,12 +825,12 @@ func ViewIssue(ctx *context.Context) { allowMerge, err = pull_service.IsUserAllowedToMerge(ctx, pull, perm, ctx.Doer) if err != nil { ctx.ServerError("IsUserAllowedToMerge", err) - return + return true } if ctx.Data["CanMarkConversation"], err = issues_model.CanMarkConversation(ctx, issue, ctx.Doer); err != nil { ctx.ServerError("CanMarkConversation", err) - return + return true } } @@ -687,10 +838,10 @@ func ViewIssue(ctx *context.Context) { ctx.Data["ShowMergeInstructions"] = canWriteToHeadRepo ctx.Data["AllowMerge"] = allowMerge - prUnit, err := repo.GetUnit(ctx, unit.TypePullRequests) + prUnit, err := issue.Repo.GetUnit(ctx, unit.TypePullRequests) if err != nil { ctx.ServerError("GetUnit", err) - return + return true } prConfig := prUnit.PullRequestsConfig() @@ -723,7 +874,7 @@ func ViewIssue(ctx *context.Context) { defaultMergeMessage, defaultMergeBody, err := pull_service.GetDefaultMergeMessage(ctx, ctx.Repo.GitRepo, pull, mergeStyle) if err != nil { ctx.ServerError("GetDefaultMergeMessage", err) - return + return true } ctx.Data["DefaultMergeMessage"] = defaultMergeMessage ctx.Data["DefaultMergeBody"] = defaultMergeBody @@ -731,7 +882,7 @@ func ViewIssue(ctx *context.Context) { defaultSquashMergeMessage, defaultSquashMergeBody, err := pull_service.GetDefaultMergeMessage(ctx, ctx.Repo.GitRepo, pull, repo_model.MergeStyleSquash) if err != nil { ctx.ServerError("GetDefaultSquashMergeMessage", err) - return + return true } ctx.Data["DefaultSquashMergeMessage"] = defaultSquashMergeMessage ctx.Data["DefaultSquashMergeBody"] = defaultSquashMergeBody @@ -739,7 +890,7 @@ func ViewIssue(ctx *context.Context) { pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pull.BaseRepoID, pull.BaseBranch) if err != nil { ctx.ServerError("LoadProtectedBranch", err) - return + return true } if pb != nil { @@ -756,38 +907,14 @@ func ViewIssue(ctx *context.Context) { ctx.Data["ChangedProtectedFilesNum"] = len(pull.ChangedProtectedFiles) ctx.Data["RequireApprovalsWhitelist"] = pb.EnableApprovalsWhitelist } - ctx.Data["WillSign"] = false - if ctx.Doer != nil { - sign, key, _, err := asymkey_service.SignMerge(ctx, pull, ctx.Doer, pull.BaseRepo.RepoPath(), pull.BaseBranch, pull.GetGitRefName()) - ctx.Data["WillSign"] = sign - ctx.Data["SigningKey"] = key - if err != nil { - if asymkey_service.IsErrWontSign(err) { - ctx.Data["WontSignReason"] = err.(*asymkey_service.ErrWontSign).Reason - } else { - ctx.Data["WontSignReason"] = "error" - log.Error("Error whilst checking if could sign pr %d in repo %s. Error: %v", pull.ID, pull.BaseRepo.FullName(), err) - } - } - } else { - ctx.Data["WontSignReason"] = "not_signed_in" - } - - isPullBranchDeletable := canDelete && - pull.HeadRepo != nil && - git.IsBranchExist(ctx, pull.HeadRepo.RepoPath(), pull.HeadBranch) && - (!pull.HasMerged || ctx.Data["HeadBranchCommitID"] == ctx.Data["PullHeadCommitID"]) - if isPullBranchDeletable && pull.HasMerged { - exist, err := issues_model.HasUnmergedPullRequestsByHeadInfo(ctx, pull.HeadRepoID, pull.HeadBranch) - if err != nil { - ctx.ServerError("HasUnmergedPullRequestsByHeadInfo", err) - return - } + if preparePullViewSigning(ctx, issue) { + return true + } - isPullBranchDeletable = !exist + if preparePullViewDeleteBranch(ctx, issue, canDelete) { + return true } - ctx.Data["IsPullBranchDeletable"] = isPullBranchDeletable stillCanManualMerge := func() bool { if pull.HasMerged || issue.IsClosed || !ctx.IsSigned { @@ -809,86 +936,24 @@ func ViewIssue(ctx *context.Context) { ctx.Data["HasPendingPullRequestMerge"], ctx.Data["PendingPullRequestMerge"], err = pull_model.GetScheduledMergeByPullID(ctx, pull.ID) if err != nil { ctx.ServerError("GetScheduledMergeByPullID", err) - return + return true } } + return false +} - // Get Dependencies - blockedBy, err := issue.BlockedByDependencies(ctx, db.ListOptions{}) - if err != nil { - ctx.ServerError("BlockedByDependencies", err) - return - } - ctx.Data["BlockedByDependencies"], ctx.Data["BlockedByDependenciesNotPermitted"] = checkBlockedByIssues(ctx, blockedBy) - if ctx.Written() { - return - } - - blocking, err := issue.BlockingDependencies(ctx) +func prepareIssueViewContent(ctx *context.Context, issue *issues_model.Issue) bool { + var err error + rctx := renderhelper.NewRenderContextRepoComment(ctx, ctx.Repo.Repository) + issue.RenderedContent, err = markdown.RenderString(rctx, issue.Content) if err != nil { - ctx.ServerError("BlockingDependencies", err) - return - } - - ctx.Data["BlockingDependencies"], ctx.Data["BlockingDependenciesNotPermitted"] = checkBlockedByIssues(ctx, blocking) - if ctx.Written() { - return + ctx.ServerError("RenderString", err) + return true } - - var pinAllowed bool - if !issue.IsPinned() { - pinAllowed, err = issues_model.IsNewPinAllowed(ctx, issue.RepoID, issue.IsPull) - if err != nil { - ctx.ServerError("IsNewPinAllowed", err) - return - } - } else { - pinAllowed = true + if issue.ShowRole, err = roleDescriptor(ctx, issue.Repo, issue.Poster, issue, issue.HasOriginalAuthor()); err != nil { + ctx.ServerError("roleDescriptor", err) + return true } - - ctx.Data["Participants"] = participants - ctx.Data["NumParticipants"] = len(participants) ctx.Data["Issue"] = issue - ctx.Data["Reference"] = issue.Ref - ctx.Data["SignInLink"] = setting.AppSubURL + "/user/login?redirect_to=" + url.QueryEscape(ctx.Data["Link"].(string)) - ctx.Data["IsIssuePoster"] = ctx.IsSigned && issue.IsPoster(ctx.Doer.ID) - ctx.Data["HasIssuesOrPullsWritePermission"] = ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) - ctx.Data["HasProjectsWritePermission"] = ctx.Repo.CanWrite(unit.TypeProjects) - ctx.Data["IsRepoAdmin"] = ctx.IsSigned && (ctx.Repo.IsAdmin() || ctx.Doer.IsAdmin) - ctx.Data["LockReasons"] = setting.Repository.Issue.LockReasons - ctx.Data["RefEndName"] = git.RefName(issue.Ref).ShortName() - ctx.Data["NewPinAllowed"] = pinAllowed - ctx.Data["PinEnabled"] = setting.Repository.Issue.MaxPinned != 0 - - var hiddenCommentTypes *big.Int - if ctx.IsSigned { - val, err := user_model.GetUserSetting(ctx, ctx.Doer.ID, user_model.SettingsKeyHiddenCommentTypes) - if err != nil { - ctx.ServerError("GetUserSetting", err) - return - } - hiddenCommentTypes, _ = new(big.Int).SetString(val, 10) // we can safely ignore the failed conversion here - } - ctx.Data["ShouldShowCommentType"] = func(commentType issues_model.CommentType) bool { - return hiddenCommentTypes == nil || hiddenCommentTypes.Bit(int(commentType)) == 0 - } - // For sidebar - PrepareBranchList(ctx) - - if ctx.Written() { - return - } - - tags, err := repo_model.GetTagNamesByRepoID(ctx, ctx.Repo.Repository.ID) - if err != nil { - ctx.ServerError("GetTagNamesByRepoID", err) - return - } - ctx.Data["Tags"] = tags - - ctx.Data["CanBlockUser"] = func(blocker, blockee *user_model.User) bool { - return user_service.CanBlockUser(ctx, ctx.Doer, blocker, blockee) - } - - ctx.HTML(http.StatusOK, tplIssueView) + return false } From 0ef0c5147caff1e325a3188cb6734de485977c88 Mon Sep 17 00:00:00 2001 From: Lunny Xiao <xiaolunwen@gmail.com> Date: Fri, 6 Dec 2024 21:58:30 -0800 Subject: [PATCH 2/5] Fix unnecessary changes --- models/issues/issue.go | 1 - 1 file changed, 1 deletion(-) diff --git a/models/issues/issue.go b/models/issues/issue.go index 8038bb9717da1..64fc20cc05e99 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -302,7 +302,6 @@ func (issue *Issue) loadReactions(ctx context.Context) (err error) { comment.Reactions = append(comment.Reactions, react) } } - issue.Reactions = reactions return nil } From c4598afb7c3529e59558ce5098e7984303735635 Mon Sep 17 00:00:00 2001 From: Lunny Xiao <xiaolunwen@gmail.com> Date: Fri, 6 Dec 2024 22:50:34 -0800 Subject: [PATCH 3/5] Refactor function signature --- routers/web/repo/issue_view.go | 135 +++++++++++++++------------------ 1 file changed, 62 insertions(+), 73 deletions(-) diff --git a/routers/web/repo/issue_view.go b/routers/web/repo/issue_view.go index d035a6dd5b66c..93655ea3618d9 100644 --- a/routers/web/repo/issue_view.go +++ b/routers/web/repo/issue_view.go @@ -374,25 +374,20 @@ func ViewIssue(ctx *context.Context) { } } - if prepareIssueViewCommentsAndSidebarParticipants(ctx, issue) { - return + prepareFuncs := []func(*context.Context, *issues_model.Issue){ + prepareIssueViewCommentsAndSidebarParticipants, + preparePullViewReviewAndMerge, + prepareIssueViewSidebarWatch, + prepareIssueViewSidebarTimeTracker, + prepareIssueViewSidebarDependency, + prepareIssueViewSidebarPin, } - if preparePullViewReviewAndMerge(ctx, issue) { - return - } - - if prepareIssueViewSidebarWatch(ctx, issue) { - return - } - if prepareIssueViewSidebarTimeTracker(ctx, issue) { - return - } - if prepareIssueViewSidebarDependency(ctx, issue) { - return - } - if prepareIssueViewSidebarPin(ctx, issue) { - return + for _, prepareFunc := range prepareFuncs { + prepareFunc(ctx, issue) + if ctx.Written() { + return + } } ctx.Data["Reference"] = issue.Ref @@ -418,7 +413,7 @@ func ViewIssue(ctx *context.Context) { ctx.HTML(http.StatusOK, tplIssueView) } -func prepareIssueViewSidebarDependency(ctx *context.Context, issue *issues_model.Issue) bool { +func prepareIssueViewSidebarDependency(ctx *context.Context, issue *issues_model.Issue) { if issue.IsPull && !ctx.Repo.CanRead(unit.TypeIssues) { ctx.Data["IssueDependencySearchType"] = "pulls" } else if !issue.IsPull && !ctx.Repo.CanRead(unit.TypePullRequests) { @@ -437,24 +432,23 @@ func prepareIssueViewSidebarDependency(ctx *context.Context, issue *issues_model blockedBy, err := issue.BlockedByDependencies(ctx, db.ListOptions{}) if err != nil { ctx.ServerError("BlockedByDependencies", err) - return true + return } ctx.Data["BlockedByDependencies"], ctx.Data["BlockedByDependenciesNotPermitted"] = checkBlockedByIssues(ctx, blockedBy) if ctx.Written() { - return true + return } blocking, err := issue.BlockingDependencies(ctx) if err != nil { ctx.ServerError("BlockingDependencies", err) - return true + return } ctx.Data["BlockingDependencies"], ctx.Data["BlockingDependenciesNotPermitted"] = checkBlockedByIssues(ctx, blocking) - return ctx.Written() } -func preparePullViewSigning(ctx *context.Context, issue *issues_model.Issue) bool { +func preparePullViewSigning(ctx *context.Context, issue *issues_model.Issue) { pull := issue.PullRequest ctx.Data["WillSign"] = false if ctx.Doer != nil { @@ -472,10 +466,9 @@ func preparePullViewSigning(ctx *context.Context, issue *issues_model.Issue) boo } else { ctx.Data["WontSignReason"] = "not_signed_in" } - return false } -func prepareIssueViewSidebarWatch(ctx *context.Context, issue *issues_model.Issue) bool { +func prepareIssueViewSidebarWatch(ctx *context.Context, issue *issues_model.Issue) { iw := new(issues_model.IssueWatch) if ctx.Doer != nil { iw.UserID = ctx.Doer.ID @@ -484,14 +477,13 @@ func prepareIssueViewSidebarWatch(ctx *context.Context, issue *issues_model.Issu iw.IsWatching, err = issues_model.CheckIssueWatch(ctx, ctx.Doer, issue) if err != nil { ctx.ServerError("CheckIssueWatch", err) - return true + return } } ctx.Data["IssueWatch"] = iw - return false } -func prepareIssueViewSidebarTimeTracker(ctx *context.Context, issue *issues_model.Issue) bool { +func prepareIssueViewSidebarTimeTracker(ctx *context.Context, issue *issues_model.Issue) { if ctx.Repo.Repository.IsTimetrackerEnabled(ctx) { if ctx.IsSigned { // Deal with the stopwatch @@ -500,7 +492,7 @@ func prepareIssueViewSidebarTimeTracker(ctx *context.Context, issue *issues_mode exists, _, swIssue, err := issues_model.HasUserStopwatch(ctx, ctx.Doer.ID) if err != nil { ctx.ServerError("HasUserStopwatch", err) - return true + return } ctx.Data["HasUserStopwatch"] = exists if exists { @@ -516,15 +508,14 @@ func prepareIssueViewSidebarTimeTracker(ctx *context.Context, issue *issues_mode var err error if ctx.Data["WorkingUsers"], err = issues_model.TotalTimesForEachUser(ctx, &issues_model.FindTrackedTimesOptions{IssueID: issue.ID}); err != nil { ctx.ServerError("TotalTimesForEachUser", err) - return true + return } } - return false } -func preparePullViewDeleteBranch(ctx *context.Context, issue *issues_model.Issue, canDelete bool) bool { +func preparePullViewDeleteBranch(ctx *context.Context, issue *issues_model.Issue, canDelete bool) { if !issue.IsPull { - return false + return } pull := issue.PullRequest isPullBranchDeletable := canDelete && @@ -536,23 +527,22 @@ func preparePullViewDeleteBranch(ctx *context.Context, issue *issues_model.Issue exist, err := issues_model.HasUnmergedPullRequestsByHeadInfo(ctx, pull.HeadRepoID, pull.HeadBranch) if err != nil { ctx.ServerError("HasUnmergedPullRequestsByHeadInfo", err) - return true + return } isPullBranchDeletable = !exist } ctx.Data["IsPullBranchDeletable"] = isPullBranchDeletable - return false } -func prepareIssueViewSidebarPin(ctx *context.Context, issue *issues_model.Issue) bool { +func prepareIssueViewSidebarPin(ctx *context.Context, issue *issues_model.Issue) { var pinAllowed bool if !issue.IsPinned() { var err error pinAllowed, err = issues_model.IsNewPinAllowed(ctx, issue.RepoID, issue.IsPull) if err != nil { ctx.ServerError("IsNewPinAllowed", err) - return true + return } } else { pinAllowed = true @@ -560,10 +550,9 @@ func prepareIssueViewSidebarPin(ctx *context.Context, issue *issues_model.Issue) ctx.Data["NewPinAllowed"] = pinAllowed ctx.Data["PinEnabled"] = setting.Repository.Issue.MaxPinned != 0 - return false } -func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue *issues_model.Issue) bool { +func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue *issues_model.Issue) { var ( role issues_model.RoleDescriptor ok bool @@ -581,11 +570,11 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue if err := issue.Comments.LoadAttachmentsByIssue(ctx); err != nil { ctx.ServerError("LoadAttachmentsByIssue", err) - return true + return } if err := issue.Comments.LoadPosters(ctx); err != nil { ctx.ServerError("LoadPosters", err) - return true + return } for _, comment = range issue.Comments { @@ -596,7 +585,7 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue comment.RenderedContent, err = markdown.RenderString(rctx, comment.Content) if err != nil { ctx.ServerError("RenderString", err) - return true + return } // Check tag. role, ok = marked[comment.PosterID] @@ -608,19 +597,19 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue comment.ShowRole, err = roleDescriptor(ctx, issue.Repo, comment.Poster, issue, comment.HasOriginalAuthor()) if err != nil { ctx.ServerError("roleDescriptor", err) - return true + return } marked[comment.PosterID] = comment.ShowRole participants = addParticipant(comment.Poster, participants) } else if comment.Type == issues_model.CommentTypeLabel { if err = comment.LoadLabel(ctx); err != nil { ctx.ServerError("LoadLabel", err) - return true + return } } else if comment.Type == issues_model.CommentTypeMilestone { if err = comment.LoadMilestone(ctx); err != nil { ctx.ServerError("LoadMilestone", err) - return true + return } ghostMilestone := &issues_model.Milestone{ ID: -1, @@ -635,7 +624,7 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue } else if comment.Type == issues_model.CommentTypeProject { if err = comment.LoadProject(ctx); err != nil { ctx.ServerError("LoadProject", err) - return true + return } ghostProject := &project_model.Project{ @@ -653,18 +642,18 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue } else if comment.Type == issues_model.CommentTypeProjectColumn { if err = comment.LoadProject(ctx); err != nil { ctx.ServerError("LoadProject", err) - return true + return } } else if comment.Type == issues_model.CommentTypeAssignees || comment.Type == issues_model.CommentTypeReviewRequest { if err = comment.LoadAssigneeUserAndTeam(ctx); err != nil { ctx.ServerError("LoadAssigneeUserAndTeam", err) - return true + return } } else if comment.Type == issues_model.CommentTypeRemoveDependency || comment.Type == issues_model.CommentTypeAddDependency { if err = comment.LoadDepIssueDetails(ctx); err != nil { if !issues_model.IsErrIssueNotExist(err) { ctx.ServerError("LoadDepIssueDetails", err) - return true + return } } } else if comment.Type.HasContentSupport() { @@ -672,11 +661,11 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue comment.RenderedContent, err = markdown.RenderString(rctx, comment.Content) if err != nil { ctx.ServerError("RenderString", err) - return true + return } if err = comment.LoadReview(ctx); err != nil && !issues_model.IsErrReviewNotExist(err) { ctx.ServerError("LoadReview", err) - return true + return } participants = addParticipant(comment.Poster, participants) if comment.Review == nil { @@ -685,13 +674,13 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue if err = comment.Review.LoadAttributes(ctx); err != nil { if !user_model.IsErrUserNotExist(err) { ctx.ServerError("Review.LoadAttributes", err) - return true + return } comment.Review.Reviewer = user_model.NewGhostUser() } if err = comment.Review.LoadCodeComments(ctx); err != nil { ctx.ServerError("Review.LoadCodeComments", err) - return true + return } for _, codeComments := range comment.Review.CodeComments { for _, lineComments := range codeComments { @@ -706,7 +695,7 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue c.ShowRole, err = roleDescriptor(ctx, issue.Repo, c.Poster, issue, c.HasOriginalAuthor()) if err != nil { ctx.ServerError("roleDescriptor", err) - return true + return } marked[c.PosterID] = c.ShowRole participants = addParticipant(c.Poster, participants) @@ -715,13 +704,13 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue } if err = comment.LoadResolveDoer(ctx); err != nil { ctx.ServerError("LoadResolveDoer", err) - return true + return } } else if comment.Type == issues_model.CommentTypePullRequestPush { participants = addParticipant(comment.Poster, participants) if err = comment.LoadPushCommits(ctx); err != nil { ctx.ServerError("LoadPushCommits", err) - return true + return } if !ctx.Repo.CanRead(unit.TypeActions) { for _, commit := range comment.Commits { @@ -765,7 +754,7 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue val, err := user_model.GetUserSetting(ctx, ctx.Doer.ID, user_model.SettingsKeyHiddenCommentTypes) if err != nil { ctx.ServerError("GetUserSetting", err) - return true + return } hiddenCommentTypes, _ = new(big.Int).SetString(val, 10) // we can safely ignore the failed conversion here } @@ -776,10 +765,9 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue // prepare for sidebar participants ctx.Data["Participants"] = participants ctx.Data["NumParticipants"] = len(participants) - return false } -func preparePullViewReviewAndMerge(ctx *context.Context, issue *issues_model.Issue) bool { +func preparePullViewReviewAndMerge(ctx *context.Context, issue *issues_model.Issue) { getBranchData(ctx, issue) if issue.IsPull { pull := issue.PullRequest @@ -795,7 +783,7 @@ func preparePullViewReviewAndMerge(ctx *context.Context, issue *issues_model.Iss perm, err := access_model.GetUserRepoPermission(ctx, pull.HeadRepo, ctx.Doer) if err != nil { ctx.ServerError("GetUserRepoPermission", err) - return true + return } if perm.CanWrite(unit.TypeCode) { // Check if branch is not protected @@ -817,7 +805,7 @@ func preparePullViewReviewAndMerge(ctx *context.Context, issue *issues_model.Iss perm, err := access_model.GetUserRepoPermission(ctx, pull.BaseRepo, ctx.Doer) if err != nil { ctx.ServerError("GetUserRepoPermission", err) - return true + return } if !canWriteToHeadRepo { // maintainers maybe allowed to push to head repo even if they can't write to it canWriteToHeadRepo = pull.AllowMaintainerEdit && perm.CanWrite(unit.TypeCode) @@ -825,12 +813,12 @@ func preparePullViewReviewAndMerge(ctx *context.Context, issue *issues_model.Iss allowMerge, err = pull_service.IsUserAllowedToMerge(ctx, pull, perm, ctx.Doer) if err != nil { ctx.ServerError("IsUserAllowedToMerge", err) - return true + return } if ctx.Data["CanMarkConversation"], err = issues_model.CanMarkConversation(ctx, issue, ctx.Doer); err != nil { ctx.ServerError("CanMarkConversation", err) - return true + return } } @@ -841,7 +829,7 @@ func preparePullViewReviewAndMerge(ctx *context.Context, issue *issues_model.Iss prUnit, err := issue.Repo.GetUnit(ctx, unit.TypePullRequests) if err != nil { ctx.ServerError("GetUnit", err) - return true + return } prConfig := prUnit.PullRequestsConfig() @@ -874,7 +862,7 @@ func preparePullViewReviewAndMerge(ctx *context.Context, issue *issues_model.Iss defaultMergeMessage, defaultMergeBody, err := pull_service.GetDefaultMergeMessage(ctx, ctx.Repo.GitRepo, pull, mergeStyle) if err != nil { ctx.ServerError("GetDefaultMergeMessage", err) - return true + return } ctx.Data["DefaultMergeMessage"] = defaultMergeMessage ctx.Data["DefaultMergeBody"] = defaultMergeBody @@ -882,7 +870,7 @@ func preparePullViewReviewAndMerge(ctx *context.Context, issue *issues_model.Iss defaultSquashMergeMessage, defaultSquashMergeBody, err := pull_service.GetDefaultMergeMessage(ctx, ctx.Repo.GitRepo, pull, repo_model.MergeStyleSquash) if err != nil { ctx.ServerError("GetDefaultSquashMergeMessage", err) - return true + return } ctx.Data["DefaultSquashMergeMessage"] = defaultSquashMergeMessage ctx.Data["DefaultSquashMergeBody"] = defaultSquashMergeBody @@ -890,7 +878,7 @@ func preparePullViewReviewAndMerge(ctx *context.Context, issue *issues_model.Iss pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pull.BaseRepoID, pull.BaseBranch) if err != nil { ctx.ServerError("LoadProtectedBranch", err) - return true + return } if pb != nil { @@ -908,12 +896,14 @@ func preparePullViewReviewAndMerge(ctx *context.Context, issue *issues_model.Iss ctx.Data["RequireApprovalsWhitelist"] = pb.EnableApprovalsWhitelist } - if preparePullViewSigning(ctx, issue) { - return true + preparePullViewSigning(ctx, issue) + if ctx.Written() { + return } - if preparePullViewDeleteBranch(ctx, issue, canDelete) { - return true + preparePullViewDeleteBranch(ctx, issue, canDelete) + if ctx.Written() { + return } stillCanManualMerge := func() bool { @@ -936,10 +926,9 @@ func preparePullViewReviewAndMerge(ctx *context.Context, issue *issues_model.Iss ctx.Data["HasPendingPullRequestMerge"], ctx.Data["PendingPullRequestMerge"], err = pull_model.GetScheduledMergeByPullID(ctx, pull.ID) if err != nil { ctx.ServerError("GetScheduledMergeByPullID", err) - return true + return } } - return false } func prepareIssueViewContent(ctx *context.Context, issue *issues_model.Issue) bool { From a1b92281e728573b6d2b27484af450813f0c1aac Mon Sep 17 00:00:00 2001 From: Lunny Xiao <xiaolunwen@gmail.com> Date: Fri, 6 Dec 2024 23:14:53 -0800 Subject: [PATCH 4/5] More refactor --- routers/web/repo/issue_view.go | 49 +++++++++++++++------------------- routers/web/repo/pull.go | 38 ++++++++++++++------------ 2 files changed, 42 insertions(+), 45 deletions(-) diff --git a/routers/web/repo/issue_view.go b/routers/web/repo/issue_view.go index 93655ea3618d9..f3a67fd1d51af 100644 --- a/routers/web/repo/issue_view.go +++ b/routers/web/repo/issue_view.go @@ -341,40 +341,25 @@ func ViewIssue(ctx *context.Context) { ctx.Data["Title"] = fmt.Sprintf("#%d - %s", issue.Index, emoji.ReplaceAliases(issue.Title)) - repo := ctx.Repo.Repository - if prepareIssueViewContent(ctx, issue) { - return - } - - // Get more information if it's a pull request. - if issue.IsPull { - if issue.PullRequest.HasMerged { - ctx.Data["DisableStatusChange"] = issue.PullRequest.HasMerged - PrepareMergedViewPullInfo(ctx, issue) - } else { - PrepareViewPullInfo(ctx, issue) - ctx.Data["DisableStatusChange"] = ctx.Data["IsPullRequestBroken"] == true && issue.IsClosed - } - if ctx.Written() { + if ctx.IsSigned { + // Update issue-user. + if err = activities_model.SetIssueReadBy(ctx, issue.ID, ctx.Doer.ID); err != nil { + ctx.ServerError("ReadBy", err) return } } - pageMetaData := retrieveRepoIssueMetaData(ctx, repo, issue, issue.IsPull) + pageMetaData := retrieveRepoIssueMetaData(ctx, ctx.Repo.Repository, issue, issue.IsPull) if ctx.Written() { return } pageMetaData.LabelsData.SetSelectedLabels(issue.Labels) - if ctx.IsSigned { - // Update issue-user. - if err = activities_model.SetIssueReadBy(ctx, issue.ID, ctx.Doer.ID); err != nil { - ctx.ServerError("ReadBy", err) - return - } - } - prepareFuncs := []func(*context.Context, *issues_model.Issue){ + prepareIssueViewContent, + func(ctx *context.Context, issue *issues_model.Issue) { + preparePullViewPullInfo(ctx, issue) + }, prepareIssueViewCommentsAndSidebarParticipants, preparePullViewReviewAndMerge, prepareIssueViewSidebarWatch, @@ -390,6 +375,15 @@ func ViewIssue(ctx *context.Context) { } } + // Get more information if it's a pull request. + if issue.IsPull { + if issue.PullRequest.HasMerged { + ctx.Data["DisableStatusChange"] = issue.PullRequest.HasMerged + } else { + ctx.Data["DisableStatusChange"] = ctx.Data["IsPullRequestBroken"] == true && issue.IsClosed + } + } + ctx.Data["Reference"] = issue.Ref ctx.Data["SignInLink"] = setting.AppSubURL + "/user/login?redirect_to=" + url.QueryEscape(ctx.Data["Link"].(string)) ctx.Data["IsIssuePoster"] = ctx.IsSigned && issue.IsPoster(ctx.Doer.ID) @@ -931,18 +925,17 @@ func preparePullViewReviewAndMerge(ctx *context.Context, issue *issues_model.Iss } } -func prepareIssueViewContent(ctx *context.Context, issue *issues_model.Issue) bool { +func prepareIssueViewContent(ctx *context.Context, issue *issues_model.Issue) { var err error rctx := renderhelper.NewRenderContextRepoComment(ctx, ctx.Repo.Repository) issue.RenderedContent, err = markdown.RenderString(rctx, issue.Content) if err != nil { ctx.ServerError("RenderString", err) - return true + return } if issue.ShowRole, err = roleDescriptor(ctx, issue.Repo, issue.Poster, issue, issue.HasOriginalAuthor()); err != nil { ctx.ServerError("roleDescriptor", err) - return true + return } ctx.Data["Issue"] = issue - return false } diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 0325585e5317e..9694ae845b1b9 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -263,8 +263,18 @@ func GetMergedBaseCommitID(ctx *context.Context, issue *issues_model.Issue) stri return baseCommit } -// PrepareMergedViewPullInfo show meta information for a merged pull request view page -func PrepareMergedViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.CompareInfo { +func preparePullViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.CompareInfo { + if !issue.IsPull { + return nil + } + if issue.PullRequest.HasMerged { + return prepareMergedViewPullInfo(ctx, issue) + } + return prepareViewPullInfo(ctx, issue) +} + +// prepareMergedViewPullInfo show meta information for a merged pull request view page +func prepareMergedViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.CompareInfo { pull := issue.PullRequest setMergeTarget(ctx, pull) @@ -309,8 +319,8 @@ func PrepareMergedViewPullInfo(ctx *context.Context, issue *issues_model.Issue) return compareInfo } -// PrepareViewPullInfo show meta information for a pull request preview page -func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.CompareInfo { +// prepareViewPullInfo show meta information for a pull request preview page +func prepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.CompareInfo { ctx.Data["PullRequestWorkInProgressPrefixes"] = setting.Repository.PullRequest.WorkInProgressPrefixes repo := ctx.Repo.Repository @@ -610,15 +620,8 @@ func ViewPullCommits(ctx *context.Context) { if !ok { return } - pull := issue.PullRequest - - var prInfo *git.CompareInfo - if pull.HasMerged { - prInfo = PrepareMergedViewPullInfo(ctx, issue) - } else { - prInfo = PrepareViewPullInfo(ctx, issue) - } + prInfo := preparePullViewPullInfo(ctx, issue) if ctx.Written() { return } else if prInfo == nil { @@ -662,11 +665,12 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi gitRepo = ctx.Repo.GitRepo ) - var prInfo *git.CompareInfo - if pull.HasMerged { - prInfo = PrepareMergedViewPullInfo(ctx, issue) - } else { - prInfo = PrepareViewPullInfo(ctx, issue) + prInfo := preparePullViewPullInfo(ctx, issue) + if ctx.Written() { + return + } else if prInfo == nil { + ctx.NotFound("ViewPullFiles", nil) + return } // Validate the given commit sha to show (if any passed) From a884256a0101e0095727f466c08949ddbc1f202e Mon Sep 17 00:00:00 2001 From: Lunny Xiao <xiaolunwen@gmail.com> Date: Fri, 6 Dec 2024 23:24:02 -0800 Subject: [PATCH 5/5] small improvements --- routers/web/repo/issue_view.go | 315 +++++++++++++++++---------------- 1 file changed, 161 insertions(+), 154 deletions(-) diff --git a/routers/web/repo/issue_view.go b/routers/web/repo/issue_view.go index f3a67fd1d51af..09b57f4e7833a 100644 --- a/routers/web/repo/issue_view.go +++ b/routers/web/repo/issue_view.go @@ -443,6 +443,9 @@ func prepareIssueViewSidebarDependency(ctx *context.Context, issue *issues_model } func preparePullViewSigning(ctx *context.Context, issue *issues_model.Issue) { + if !issue.IsPull { + return + } pull := issue.PullRequest ctx.Data["WillSign"] = false if ctx.Doer != nil { @@ -478,32 +481,34 @@ func prepareIssueViewSidebarWatch(ctx *context.Context, issue *issues_model.Issu } func prepareIssueViewSidebarTimeTracker(ctx *context.Context, issue *issues_model.Issue) { - if ctx.Repo.Repository.IsTimetrackerEnabled(ctx) { - if ctx.IsSigned { - // Deal with the stopwatch - ctx.Data["IsStopwatchRunning"] = issues_model.StopwatchExists(ctx, ctx.Doer.ID, issue.ID) - if !ctx.Data["IsStopwatchRunning"].(bool) { - exists, _, swIssue, err := issues_model.HasUserStopwatch(ctx, ctx.Doer.ID) - if err != nil { - ctx.ServerError("HasUserStopwatch", err) - return - } - ctx.Data["HasUserStopwatch"] = exists - if exists { - // Add warning if the user has already a stopwatch - // Add link to the issue of the already running stopwatch - ctx.Data["OtherStopwatchURL"] = swIssue.Link() - } + if !ctx.Repo.Repository.IsTimetrackerEnabled(ctx) { + return + } + + if ctx.IsSigned { + // Deal with the stopwatch + ctx.Data["IsStopwatchRunning"] = issues_model.StopwatchExists(ctx, ctx.Doer.ID, issue.ID) + if !ctx.Data["IsStopwatchRunning"].(bool) { + exists, _, swIssue, err := issues_model.HasUserStopwatch(ctx, ctx.Doer.ID) + if err != nil { + ctx.ServerError("HasUserStopwatch", err) + return + } + ctx.Data["HasUserStopwatch"] = exists + if exists { + // Add warning if the user has already a stopwatch + // Add link to the issue of the already running stopwatch + ctx.Data["OtherStopwatchURL"] = swIssue.Link() } - ctx.Data["CanUseTimetracker"] = ctx.Repo.CanUseTimetracker(ctx, issue, ctx.Doer) - } else { - ctx.Data["CanUseTimetracker"] = false - } - var err error - if ctx.Data["WorkingUsers"], err = issues_model.TotalTimesForEachUser(ctx, &issues_model.FindTrackedTimesOptions{IssueID: issue.ID}); err != nil { - ctx.ServerError("TotalTimesForEachUser", err) - return } + ctx.Data["CanUseTimetracker"] = ctx.Repo.CanUseTimetracker(ctx, issue, ctx.Doer) + } else { + ctx.Data["CanUseTimetracker"] = false + } + var err error + if ctx.Data["WorkingUsers"], err = issues_model.TotalTimesForEachUser(ctx, &issues_model.FindTrackedTimesOptions{IssueID: issue.ID}); err != nil { + ctx.ServerError("TotalTimesForEachUser", err) + return } } @@ -763,165 +768,167 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue func preparePullViewReviewAndMerge(ctx *context.Context, issue *issues_model.Issue) { getBranchData(ctx, issue) - if issue.IsPull { - pull := issue.PullRequest - pull.Issue = issue - canDelete := false - allowMerge := false - canWriteToHeadRepo := false - - if ctx.IsSigned { - if err := pull.LoadHeadRepo(ctx); err != nil { - log.Error("LoadHeadRepo: %v", err) - } else if pull.HeadRepo != nil { - perm, err := access_model.GetUserRepoPermission(ctx, pull.HeadRepo, ctx.Doer) - if err != nil { - ctx.ServerError("GetUserRepoPermission", err) - return - } - if perm.CanWrite(unit.TypeCode) { - // Check if branch is not protected - if pull.HeadBranch != pull.HeadRepo.DefaultBranch { - if protected, err := git_model.IsBranchProtected(ctx, pull.HeadRepo.ID, pull.HeadBranch); err != nil { - log.Error("IsProtectedBranch: %v", err) - } else if !protected { - canDelete = true - ctx.Data["DeleteBranchLink"] = issue.Link() + "/cleanup" - } - } - canWriteToHeadRepo = true - } - } + if !issue.IsPull { + return + } - if err := pull.LoadBaseRepo(ctx); err != nil { - log.Error("LoadBaseRepo: %v", err) - } - perm, err := access_model.GetUserRepoPermission(ctx, pull.BaseRepo, ctx.Doer) + pull := issue.PullRequest + pull.Issue = issue + canDelete := false + allowMerge := false + canWriteToHeadRepo := false + + if ctx.IsSigned { + if err := pull.LoadHeadRepo(ctx); err != nil { + log.Error("LoadHeadRepo: %v", err) + } else if pull.HeadRepo != nil { + perm, err := access_model.GetUserRepoPermission(ctx, pull.HeadRepo, ctx.Doer) if err != nil { ctx.ServerError("GetUserRepoPermission", err) return } - if !canWriteToHeadRepo { // maintainers maybe allowed to push to head repo even if they can't write to it - canWriteToHeadRepo = pull.AllowMaintainerEdit && perm.CanWrite(unit.TypeCode) - } - allowMerge, err = pull_service.IsUserAllowedToMerge(ctx, pull, perm, ctx.Doer) - if err != nil { - ctx.ServerError("IsUserAllowedToMerge", err) - return - } - - if ctx.Data["CanMarkConversation"], err = issues_model.CanMarkConversation(ctx, issue, ctx.Doer); err != nil { - ctx.ServerError("CanMarkConversation", err) - return + if perm.CanWrite(unit.TypeCode) { + // Check if branch is not protected + if pull.HeadBranch != pull.HeadRepo.DefaultBranch { + if protected, err := git_model.IsBranchProtected(ctx, pull.HeadRepo.ID, pull.HeadBranch); err != nil { + log.Error("IsProtectedBranch: %v", err) + } else if !protected { + canDelete = true + ctx.Data["DeleteBranchLink"] = issue.Link() + "/cleanup" + } + } + canWriteToHeadRepo = true } } - ctx.Data["CanWriteToHeadRepo"] = canWriteToHeadRepo - ctx.Data["ShowMergeInstructions"] = canWriteToHeadRepo - ctx.Data["AllowMerge"] = allowMerge - - prUnit, err := issue.Repo.GetUnit(ctx, unit.TypePullRequests) + if err := pull.LoadBaseRepo(ctx); err != nil { + log.Error("LoadBaseRepo: %v", err) + } + perm, err := access_model.GetUserRepoPermission(ctx, pull.BaseRepo, ctx.Doer) if err != nil { - ctx.ServerError("GetUnit", err) + ctx.ServerError("GetUserRepoPermission", err) return } - prConfig := prUnit.PullRequestsConfig() - - ctx.Data["AutodetectManualMerge"] = prConfig.AutodetectManualMerge - - var mergeStyle repo_model.MergeStyle - // Check correct values and select default - if ms, ok := ctx.Data["MergeStyle"].(repo_model.MergeStyle); !ok || - !prConfig.IsMergeStyleAllowed(ms) { - defaultMergeStyle := prConfig.GetDefaultMergeStyle() - if prConfig.IsMergeStyleAllowed(defaultMergeStyle) && !ok { - mergeStyle = defaultMergeStyle - } else if prConfig.AllowMerge { - mergeStyle = repo_model.MergeStyleMerge - } else if prConfig.AllowRebase { - mergeStyle = repo_model.MergeStyleRebase - } else if prConfig.AllowRebaseMerge { - mergeStyle = repo_model.MergeStyleRebaseMerge - } else if prConfig.AllowSquash { - mergeStyle = repo_model.MergeStyleSquash - } else if prConfig.AllowFastForwardOnly { - mergeStyle = repo_model.MergeStyleFastForwardOnly - } else if prConfig.AllowManualMerge { - mergeStyle = repo_model.MergeStyleManuallyMerged - } + if !canWriteToHeadRepo { // maintainers maybe allowed to push to head repo even if they can't write to it + canWriteToHeadRepo = pull.AllowMaintainerEdit && perm.CanWrite(unit.TypeCode) } - - ctx.Data["MergeStyle"] = mergeStyle - - defaultMergeMessage, defaultMergeBody, err := pull_service.GetDefaultMergeMessage(ctx, ctx.Repo.GitRepo, pull, mergeStyle) + allowMerge, err = pull_service.IsUserAllowedToMerge(ctx, pull, perm, ctx.Doer) if err != nil { - ctx.ServerError("GetDefaultMergeMessage", err) + ctx.ServerError("IsUserAllowedToMerge", err) return } - ctx.Data["DefaultMergeMessage"] = defaultMergeMessage - ctx.Data["DefaultMergeBody"] = defaultMergeBody - defaultSquashMergeMessage, defaultSquashMergeBody, err := pull_service.GetDefaultMergeMessage(ctx, ctx.Repo.GitRepo, pull, repo_model.MergeStyleSquash) - if err != nil { - ctx.ServerError("GetDefaultSquashMergeMessage", err) + if ctx.Data["CanMarkConversation"], err = issues_model.CanMarkConversation(ctx, issue, ctx.Doer); err != nil { + ctx.ServerError("CanMarkConversation", err) return } - ctx.Data["DefaultSquashMergeMessage"] = defaultSquashMergeMessage - ctx.Data["DefaultSquashMergeBody"] = defaultSquashMergeBody + } - pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pull.BaseRepoID, pull.BaseBranch) - if err != nil { - ctx.ServerError("LoadProtectedBranch", err) - return - } + ctx.Data["CanWriteToHeadRepo"] = canWriteToHeadRepo + ctx.Data["ShowMergeInstructions"] = canWriteToHeadRepo + ctx.Data["AllowMerge"] = allowMerge - if pb != nil { - pb.Repo = pull.BaseRepo - ctx.Data["ProtectedBranch"] = pb - ctx.Data["IsBlockedByApprovals"] = !issues_model.HasEnoughApprovals(ctx, pb, pull) - ctx.Data["IsBlockedByRejection"] = issues_model.MergeBlockedByRejectedReview(ctx, pb, pull) - ctx.Data["IsBlockedByOfficialReviewRequests"] = issues_model.MergeBlockedByOfficialReviewRequests(ctx, pb, pull) - ctx.Data["IsBlockedByOutdatedBranch"] = issues_model.MergeBlockedByOutdatedBranch(pb, pull) - ctx.Data["GrantedApprovals"] = issues_model.GetGrantedApprovalsCount(ctx, pb, pull) - ctx.Data["RequireSigned"] = pb.RequireSignedCommits - ctx.Data["ChangedProtectedFiles"] = pull.ChangedProtectedFiles - ctx.Data["IsBlockedByChangedProtectedFiles"] = len(pull.ChangedProtectedFiles) != 0 - ctx.Data["ChangedProtectedFilesNum"] = len(pull.ChangedProtectedFiles) - ctx.Data["RequireApprovalsWhitelist"] = pb.EnableApprovalsWhitelist - } + prUnit, err := issue.Repo.GetUnit(ctx, unit.TypePullRequests) + if err != nil { + ctx.ServerError("GetUnit", err) + return + } + prConfig := prUnit.PullRequestsConfig() - preparePullViewSigning(ctx, issue) - if ctx.Written() { - return - } + ctx.Data["AutodetectManualMerge"] = prConfig.AutodetectManualMerge - preparePullViewDeleteBranch(ctx, issue, canDelete) - if ctx.Written() { - return + var mergeStyle repo_model.MergeStyle + // Check correct values and select default + if ms, ok := ctx.Data["MergeStyle"].(repo_model.MergeStyle); !ok || + !prConfig.IsMergeStyleAllowed(ms) { + defaultMergeStyle := prConfig.GetDefaultMergeStyle() + if prConfig.IsMergeStyleAllowed(defaultMergeStyle) && !ok { + mergeStyle = defaultMergeStyle + } else if prConfig.AllowMerge { + mergeStyle = repo_model.MergeStyleMerge + } else if prConfig.AllowRebase { + mergeStyle = repo_model.MergeStyleRebase + } else if prConfig.AllowRebaseMerge { + mergeStyle = repo_model.MergeStyleRebaseMerge + } else if prConfig.AllowSquash { + mergeStyle = repo_model.MergeStyleSquash + } else if prConfig.AllowFastForwardOnly { + mergeStyle = repo_model.MergeStyleFastForwardOnly + } else if prConfig.AllowManualMerge { + mergeStyle = repo_model.MergeStyleManuallyMerged } + } - stillCanManualMerge := func() bool { - if pull.HasMerged || issue.IsClosed || !ctx.IsSigned { - return false - } - if pull.CanAutoMerge() || pull.IsWorkInProgress(ctx) || pull.IsChecking() { - return false - } - if allowMerge && prConfig.AllowManualMerge { - return true - } + ctx.Data["MergeStyle"] = mergeStyle + + defaultMergeMessage, defaultMergeBody, err := pull_service.GetDefaultMergeMessage(ctx, ctx.Repo.GitRepo, pull, mergeStyle) + if err != nil { + ctx.ServerError("GetDefaultMergeMessage", err) + return + } + ctx.Data["DefaultMergeMessage"] = defaultMergeMessage + ctx.Data["DefaultMergeBody"] = defaultMergeBody + + defaultSquashMergeMessage, defaultSquashMergeBody, err := pull_service.GetDefaultMergeMessage(ctx, ctx.Repo.GitRepo, pull, repo_model.MergeStyleSquash) + if err != nil { + ctx.ServerError("GetDefaultSquashMergeMessage", err) + return + } + ctx.Data["DefaultSquashMergeMessage"] = defaultSquashMergeMessage + ctx.Data["DefaultSquashMergeBody"] = defaultSquashMergeBody + + pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pull.BaseRepoID, pull.BaseBranch) + if err != nil { + ctx.ServerError("LoadProtectedBranch", err) + return + } + + if pb != nil { + pb.Repo = pull.BaseRepo + ctx.Data["ProtectedBranch"] = pb + ctx.Data["IsBlockedByApprovals"] = !issues_model.HasEnoughApprovals(ctx, pb, pull) + ctx.Data["IsBlockedByRejection"] = issues_model.MergeBlockedByRejectedReview(ctx, pb, pull) + ctx.Data["IsBlockedByOfficialReviewRequests"] = issues_model.MergeBlockedByOfficialReviewRequests(ctx, pb, pull) + ctx.Data["IsBlockedByOutdatedBranch"] = issues_model.MergeBlockedByOutdatedBranch(pb, pull) + ctx.Data["GrantedApprovals"] = issues_model.GetGrantedApprovalsCount(ctx, pb, pull) + ctx.Data["RequireSigned"] = pb.RequireSignedCommits + ctx.Data["ChangedProtectedFiles"] = pull.ChangedProtectedFiles + ctx.Data["IsBlockedByChangedProtectedFiles"] = len(pull.ChangedProtectedFiles) != 0 + ctx.Data["ChangedProtectedFilesNum"] = len(pull.ChangedProtectedFiles) + ctx.Data["RequireApprovalsWhitelist"] = pb.EnableApprovalsWhitelist + } + + preparePullViewSigning(ctx, issue) + if ctx.Written() { + return + } + + preparePullViewDeleteBranch(ctx, issue, canDelete) + if ctx.Written() { + return + } + stillCanManualMerge := func() bool { + if pull.HasMerged || issue.IsClosed || !ctx.IsSigned { return false } + if pull.CanAutoMerge() || pull.IsWorkInProgress(ctx) || pull.IsChecking() { + return false + } + if allowMerge && prConfig.AllowManualMerge { + return true + } - ctx.Data["StillCanManualMerge"] = stillCanManualMerge() + return false + } - // Check if there is a pending pr merge - ctx.Data["HasPendingPullRequestMerge"], ctx.Data["PendingPullRequestMerge"], err = pull_model.GetScheduledMergeByPullID(ctx, pull.ID) - if err != nil { - ctx.ServerError("GetScheduledMergeByPullID", err) - return - } + ctx.Data["StillCanManualMerge"] = stillCanManualMerge() + + // Check if there is a pending pr merge + ctx.Data["HasPendingPullRequestMerge"], ctx.Data["PendingPullRequestMerge"], err = pull_model.GetScheduledMergeByPullID(ctx, pull.ID) + if err != nil { + ctx.ServerError("GetScheduledMergeByPullID", err) + return } }