Split issue/pull view router function as multiple smaller functions (#32749)

This PR splits issue/pull view function into multiple smaller functions.
It also removed duplicated branches load.

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,46 +341,6 @@ 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)
-		return
-	}
-
-	repo := ctx.Repo.Repository
-
-	// 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() {
-			return
-		}
-	}
-
-	pageMetaData := retrieveRepoIssueMetaData(ctx, repo, 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 {
@@ -397,22 +349,148 @@ func ViewIssue(ctx *context.Context) {
 		}
 	}
 
-	var (
+	pageMetaData := retrieveRepoIssueMetaData(ctx, ctx.Repo.Repository, issue, issue.IsPull)
-		role                 issues_model.RoleDescriptor
+	if ctx.Written() {
-		ok                   bool
+		return
-		marked               = make(map[int64]issues_model.RoleDescriptor)
+	}
-		comment              *issues_model.Comment
+	pageMetaData.LabelsData.SetSelectedLabels(issue.Labels)
-		participants         = make([]*user_model.User, 1, 10)
+
-		latestCloseCommentID int64
+	prepareFuncs := []func(*context.Context, *issues_model.Issue){
-	)
+		prepareIssueViewContent,
-	if ctx.Repo.Repository.IsTimetrackerEnabled(ctx) {
+		func(ctx *context.Context, issue *issues_model.Issue) {
+			preparePullViewPullInfo(ctx, issue)
+		},
+		prepareIssueViewCommentsAndSidebarParticipants,
+		preparePullViewReviewAndMerge,
+		prepareIssueViewSidebarWatch,
+		prepareIssueViewSidebarTimeTracker,
+		prepareIssueViewSidebarDependency,
+		prepareIssueViewSidebarPin,
+	}
+
+	for _, prepareFunc := range prepareFuncs {
+		prepareFunc(ctx, issue)
+		if ctx.Written() {
+			return
+		}
+	}
+
+	// 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)
+	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) {
+	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
+	}
+	ctx.Data["BlockedByDependencies"], ctx.Data["BlockedByDependenciesNotPermitted"] = checkBlockedByIssues(ctx, blockedBy)
+	if ctx.Written() {
+		return
+	}
+
+	blocking, err := issue.BlockingDependencies(ctx)
+	if err != nil {
+		ctx.ServerError("BlockingDependencies", err)
+		return
+	}
+
+	ctx.Data["BlockingDependencies"], ctx.Data["BlockingDependenciesNotPermitted"] = checkBlockedByIssues(ctx, blocking)
+}
+
+func preparePullViewSigning(ctx *context.Context, issue *issues_model.Issue) {
+	if !issue.IsPull {
+		return
+	}
+	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"
+	}
+}
+
+func prepareIssueViewSidebarWatch(ctx *context.Context, issue *issues_model.Issue) {
+	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
+		}
+	}
+	ctx.Data["IssueWatch"] = iw
+}
+
+func prepareIssueViewSidebarTimeTracker(ctx *context.Context, issue *issues_model.Issue) {
+	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) {
-				var exists bool
+			exists, _, swIssue, err := issues_model.HasUserStopwatch(ctx, ctx.Doer.ID)
-				var swIssue *issues_model.Issue
+			if err != nil {
-				if exists, _, swIssue, err = issues_model.HasUserStopwatch(ctx, ctx.Doer.ID); err != nil {
 				ctx.ServerError("HasUserStopwatch", err)
 				return
 			}
@@ -427,22 +505,63 @@ 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
 	}
-	}
+}
 
-	// Check if the user can use the dependencies
+func preparePullViewDeleteBranch(ctx *context.Context, issue *issues_model.Issue, canDelete bool) {
-	ctx.Data["CanCreateIssueDependencies"] = ctx.Repo.CanCreateIssueDependencies(ctx, ctx.Doer, issue.IsPull)
+	if !issue.IsPull {
-
-	// check if dependencies can be created across repositories
-	ctx.Data["AllowCrossRepositoryDependencies"] = setting.Service.AllowCrossRepositoryDependencies
-
-	if issue.ShowRole, err = roleDescriptor(ctx, repo, issue.Poster, issue, issue.HasOriginalAuthor()); err != nil {
-		ctx.ServerError("roleDescriptor", err)
 		return
 	}
+	pull := issue.PullRequest
+	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
+		}
+
+		isPullBranchDeletable = !exist
+	}
+	ctx.Data["IsPullBranchDeletable"] = isPullBranchDeletable
+}
+
+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
+		}
+	} else {
+		pinAllowed = true
+	}
+
+	ctx.Data["NewPinAllowed"] = pinAllowed
+	ctx.Data["PinEnabled"] = setting.Repository.Issue.MaxPinned != 0
+}
+
+func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue *issues_model.Issue) {
+	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.
@@ -461,7 +580,7 @@ func ViewIssue(ctx *context.Context) {
 		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)
@@ -474,7 +593,7 @@ 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
@@ -537,7 +656,7 @@ func ViewIssue(ctx *context.Context) {
 				}
 			}
 		} 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)
@@ -572,7 +691,7 @@ 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
@@ -629,8 +748,30 @@ 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
+		}
+		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)
+}
+
+func preparePullViewReviewAndMerge(ctx *context.Context, issue *issues_model.Issue) {
 	getBranchData(ctx, issue)
-	if issue.IsPull {
+	if !issue.IsPull {
+		return
+	}
+
 	pull := issue.PullRequest
 	pull.Issue = issue
 	canDelete := false
@@ -687,7 +828,7 @@ 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
@@ -756,38 +897,16 @@ 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 &&
+	preparePullViewSigning(ctx, issue)
-			pull.HeadRepo != nil &&
+	if ctx.Written() {
-			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
 	}
 
-			isPullBranchDeletable = !exist
+	preparePullViewDeleteBranch(ctx, issue, canDelete)
+	if ctx.Written() {
+		return
 	}
-		ctx.Data["IsPullBranchDeletable"] = isPullBranchDeletable
 
 	stillCanManualMerge := func() bool {
 		if pull.HasMerged || issue.IsClosed || !ctx.IsSigned {
@@ -811,84 +930,19 @@ func ViewIssue(ctx *context.Context) {
 		ctx.ServerError("GetScheduledMergeByPullID", err)
 		return
 	}
-	}
+}
-
+
-	// Get Dependencies
+func prepareIssueViewContent(ctx *context.Context, issue *issues_model.Issue) {
-	blockedBy, err := issue.BlockedByDependencies(ctx, db.ListOptions{})
+	var err error
-	if err != nil {
+	rctx := renderhelper.NewRenderContextRepoComment(ctx, ctx.Repo.Repository)
-		ctx.ServerError("BlockedByDependencies", err)
+	issue.RenderedContent, err = markdown.RenderString(rctx, issue.Content)
-		return
+	if err != nil {
-	}
+		ctx.ServerError("RenderString", err)
-	ctx.Data["BlockedByDependencies"], ctx.Data["BlockedByDependenciesNotPermitted"] = checkBlockedByIssues(ctx, blockedBy)
+		return
-	if ctx.Written() {
+	}
-		return
+	if issue.ShowRole, err = roleDescriptor(ctx, issue.Repo, issue.Poster, issue, issue.HasOriginalAuthor()); err != nil {
-	}
+		ctx.ServerError("roleDescriptor", err)
-
+		return
-	blocking, err := issue.BlockingDependencies(ctx)
+	}
-	if err != nil {
+	ctx.Data["Issue"] = issue
-		ctx.ServerError("BlockingDependencies", err)
-		return
-	}
-
-	ctx.Data["BlockingDependencies"], ctx.Data["BlockingDependenciesNotPermitted"] = checkBlockedByIssues(ctx, blocking)
-	if ctx.Written() {
-		return
-	}
-
-	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
-	}
-
-	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)
 }

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 preparePullViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.CompareInfo {
-func PrepareMergedViewPullInfo(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
+// prepareViewPullInfo show meta information for a pull request preview page
-func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.CompareInfo {
+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
+	prInfo := preparePullViewPullInfo(ctx, issue)
-	if pull.HasMerged {
+	if ctx.Written() {
-		prInfo = PrepareMergedViewPullInfo(ctx, issue)
+		return
-	} else {
+	} else if prInfo == nil {
-		prInfo = PrepareViewPullInfo(ctx, issue)
+		ctx.NotFound("ViewPullFiles", nil)
+		return
 	}
 
 	// Validate the given commit sha to show (if any passed)