From 2361ec5a444f6b00da502af0c1b5ca62a87a80ae Mon Sep 17 00:00:00 2001
From: Lunny Xiao <xiaolunwen@gmail.com>
Date: Wed, 21 Aug 2024 19:08:18 -0700
Subject: [PATCH] Improvements for creating branch model

---
 models/issues/issue_dev_link.go               |  7 +-
 models/migrations/v1_23/v305.go               | 11 +--
 options/locale/locale_en-US.ini               |  2 +
 routers/web/repo/issue.go                     | 10 ++-
 routers/web/repo/issue_dev.go                 | 71 ++++++++++++++++---
 services/forms/repo_branch_form.go            |  1 +
 services/issue/dev_link.go                    | 24 ++++++-
 services/pull/pull.go                         | 49 ++++++-------
 .../repo/issue/view_content/sidebar.tmpl      |  7 +-
 .../view_content/sidebar_development.tmpl     | 16 +++--
 templates/repo/issue/view_title.tmpl          |  4 ++
 11 files changed, 149 insertions(+), 53 deletions(-)

diff --git a/models/issues/issue_dev_link.go b/models/issues/issue_dev_link.go
index cdb9e5d040..9e6c7cfc25 100644
--- a/models/issues/issue_dev_link.go
+++ b/models/issues/issue_dev_link.go
@@ -28,9 +28,10 @@ type IssueDevLink struct {
 	LinkIndex    string             // branch name, pull request number or commit sha
 	CreatedUnix  timeutil.TimeStamp `xorm:"INDEX created"`
 
-	LinkedRepo  *repo_model.Repository `xorm:"-"`
-	PullRequest *PullRequest           `xorm:"-"`
-	Branch      *git_model.Branch      `xorm:"-"`
+	LinkedRepo    *repo_model.Repository `xorm:"-"`
+	PullRequest   *PullRequest           `xorm:"-"`
+	Branch        *git_model.Branch      `xorm:"-"`
+	DisplayBranch bool                   `xorm:"-"`
 }
 
 func init() {
diff --git a/models/migrations/v1_23/v305.go b/models/migrations/v1_23/v305.go
index fe696984a3..c017890b50 100644
--- a/models/migrations/v1_23/v305.go
+++ b/models/migrations/v1_23/v305.go
@@ -11,11 +11,12 @@ import (
 
 func CreateTableIssueDevLink(x *xorm.Engine) error {
 	type IssueDevLink struct {
-		ID          int64 `xorm:"pk autoincr"`
-		IssueID     int64 `xorm:"INDEX"`
-		LinkType    int
-		LinkIndex   string             // branch name, pull request number or commit sha
-		CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"`
+		ID           int64 `xorm:"pk autoincr"`
+		IssueID      int64 `xorm:"INDEX"`
+		LinkType     int
+		LinkedRepoID int64              `xorm:"INDEX"` // it can link to self repo or other repo
+		LinkIndex    string             // branch name, pull request number or commit sha
+		CreatedUnix  timeutil.TimeStamp `xorm:"INDEX created"`
 	}
 	return x.Sync(new(IssueDevLink))
 }
diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
index 59ff860d1f..850b3e6a3d 100644
--- a/options/locale/locale_en-US.ini
+++ b/options/locale/locale_en-US.ini
@@ -1623,6 +1623,8 @@ issues.label.filter_sort.reverse_alphabetically = Reverse alphabetically
 issues.label.filter_sort.by_size = Smallest size
 issues.label.filter_sort.reverse_by_size = Largest size
 issues.development = Development
+issues.maybefixed = May be fixed by %s
+issues.create_branch_from_issue_success = Create branch %s from issue successfully
 issues.num_participants = %d Participants
 issues.attachment.open_tab = `Click to see "%s" in a new tab`
 issues.attachment.download = `Click to download "%s"`
diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go
index dc8c706332..1217a6e659 100644
--- a/routers/web/repo/issue.go
+++ b/routers/web/repo/issue.go
@@ -2085,10 +2085,18 @@ func ViewIssue(ctx *context.Context) {
 
 	devLinks, err := issue_service.FindIssueDevLinksByIssue(ctx, issue)
 	if err != nil {
-		ctx.ServerError("FindIssueDevLinksByIssueID", err)
+		ctx.ServerError("FindIssueDevLinksByIssue", err)
 		return
 	}
 	ctx.Data["DevLinks"] = devLinks
+	for _, link := range devLinks {
+		if link.LinkType == issues_model.IssueDevLinkTypePullRequest {
+			if !(link.PullRequest.Issue.IsClosed && !link.PullRequest.HasMerged) {
+				ctx.Data["MaybeFixed"] = link.PullRequest
+				break
+			}
+		}
+	}
 
 	ctx.HTML(http.StatusOK, tplIssueView)
 }
diff --git a/routers/web/repo/issue_dev.go b/routers/web/repo/issue_dev.go
index fadf59cc0b..a32c051961 100644
--- a/routers/web/repo/issue_dev.go
+++ b/routers/web/repo/issue_dev.go
@@ -4,10 +4,15 @@
 package repo
 
 import (
-	"net/http"
-
+	git_model "code.gitea.io/gitea/models/git"
 	issues_model "code.gitea.io/gitea/models/issues"
+	access_model "code.gitea.io/gitea/models/perm/access"
+	repo_model "code.gitea.io/gitea/models/repo"
+	unit_model "code.gitea.io/gitea/models/unit"
+	"code.gitea.io/gitea/modules/git"
+	"code.gitea.io/gitea/modules/gitrepo"
 	"code.gitea.io/gitea/modules/web"
+	"code.gitea.io/gitea/routers/utils"
 	"code.gitea.io/gitea/services/context"
 	"code.gitea.io/gitea/services/forms"
 	repo_service "code.gitea.io/gitea/services/repository"
@@ -21,30 +26,78 @@ func CreateBranchFromIssue(ctx *context.Context) {
 
 	if issue.IsPull {
 		ctx.Flash.Error(ctx.Tr("repo.issues.create_branch_from_issue_error_is_pull"))
-		ctx.Redirect(issue.Link(), http.StatusSeeOther)
+		ctx.JSONRedirect(issue.Link())
 		return
 	}
 
 	form := web.GetForm(ctx).(*forms.NewBranchForm)
-	if !ctx.Repo.CanCreateBranch() {
+	repo := ctx.Repo.Repository
+	gitRepo := ctx.Repo.GitRepo
+	if form.RepoID > 0 {
+		var err error
+		repo, err = repo_model.GetRepositoryByID(ctx, form.RepoID)
+		if err != nil {
+			ctx.ServerError("GetRepositoryByID", err)
+			return
+		}
+		gitRepo, err = gitrepo.OpenRepository(ctx, repo)
+		if err != nil {
+			ctx.ServerError("GetRepositoryByID", err)
+			return
+		}
+		defer gitRepo.Close()
+	}
+
+	perm, err := access_model.GetUserRepoPermission(ctx, repo, ctx.Doer)
+	if err != nil {
+		ctx.ServerError("GetRepositoryByID", err)
+		return
+	}
+
+	canCreateBranch := perm.CanWrite(unit_model.TypeCode) && repo.CanCreateBranch()
+	if !canCreateBranch {
 		ctx.NotFound("CreateBranch", nil)
 		return
 	}
 
 	if ctx.HasError() {
-		ctx.Flash.Error(ctx.GetErrMsg())
-		ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL())
+		ctx.JSONError(ctx.GetErrMsg())
 		return
 	}
 
-	if err := repo_service.CreateNewBranch(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.GitRepo, form.SourceBranchName, form.NewBranchName); err != nil {
-		handleCreateBranchError(ctx, err, form)
+	if err := repo_service.CreateNewBranch(ctx, ctx.Doer, repo, gitRepo, form.SourceBranchName, form.NewBranchName); err != nil {
+		switch {
+		case git_model.IsErrBranchAlreadyExists(err) || git.IsErrPushOutOfDate(err):
+			ctx.JSONError(ctx.Tr("repo.branch.branch_already_exists", form.NewBranchName))
+		case git_model.IsErrBranchNameConflict(err):
+			e := err.(git_model.ErrBranchNameConflict)
+			ctx.JSONError(ctx.Tr("repo.branch.branch_name_conflict", form.NewBranchName, e.BranchName))
+		case git.IsErrPushRejected(err):
+			e := err.(*git.ErrPushRejected)
+			if len(e.Message) == 0 {
+				ctx.Flash.Error(ctx.Tr("repo.editor.push_rejected_no_message"))
+			} else {
+				flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{
+					"Message": ctx.Tr("repo.editor.push_rejected"),
+					"Summary": ctx.Tr("repo.editor.push_rejected_summary"),
+					"Details": utils.SanitizeFlashErrorString(e.Message),
+				})
+				if err != nil {
+					ctx.ServerError("UpdatePullRequest.HTMLString", err)
+					return
+				}
+				ctx.JSONError(flashError)
+			}
+		default:
+			ctx.ServerError("CreateNewBranch", err)
+		}
 		return
 	}
 
 	if err := issues_model.CreateIssueDevLink(ctx, &issues_model.IssueDevLink{
 		IssueID:   issue.ID,
 		LinkType:  issues_model.IssueDevLinkTypeBranch,
+		LinkedRepoID: repo.ID,
 		LinkIndex: form.NewBranchName,
 	}); err != nil {
 		ctx.ServerError("CreateIssueDevLink", err)
@@ -52,5 +105,5 @@ func CreateBranchFromIssue(ctx *context.Context) {
 	}
 
 	ctx.Flash.Success(ctx.Tr("repo.issues.create_branch_from_issue_success", ctx.Repo.BranchName))
-	ctx.Redirect(issue.Link())
+	ctx.JSONRedirect(issue.Link())
 }
diff --git a/services/forms/repo_branch_form.go b/services/forms/repo_branch_form.go
index 32b17c7d54..16ebddf07d 100644
--- a/services/forms/repo_branch_form.go
+++ b/services/forms/repo_branch_form.go
@@ -15,6 +15,7 @@ import (
 // NewBranchForm form for creating a new branch
 type NewBranchForm struct {
 	NewBranchName    string `binding:"Required;MaxSize(100);GitRefName"`
+	RepoID           int64
 	SourceBranchName string
 	CurrentPath      string
 	CreateTag        bool
diff --git a/services/issue/dev_link.go b/services/issue/dev_link.go
index 37544444fa..1e0f4b14c9 100644
--- a/services/issue/dev_link.go
+++ b/services/issue/dev_link.go
@@ -5,11 +5,14 @@ package issue
 
 import (
 	"context"
+	"fmt"
+	"sort"
 	"strconv"
 
 	git_model "code.gitea.io/gitea/models/git"
 	issues_model "code.gitea.io/gitea/models/issues"
 	repo_model "code.gitea.io/gitea/models/repo"
+	"code.gitea.io/gitea/modules/container"
 )
 
 func FindIssueDevLinksByIssue(ctx context.Context, issue *issues_model.Issue) (issues_model.IssueDevLinks, error) {
@@ -22,6 +25,17 @@ func FindIssueDevLinksByIssue(ctx context.Context, issue *issues_model.Issue) (i
 		return nil, err
 	}
 
+	sort.Slice(devLinks, func(i, j int) bool {
+		switch {
+		case devLinks[j].LinkType == issues_model.IssueDevLinkTypePullRequest:
+			return false
+		default:
+			return true
+		}
+	})
+
+	branchPRExists := make(container.Set[string])
+
 	for _, link := range devLinks {
 		if link.LinkedRepoID == 0 {
 			link.LinkedRepoID = issue.RepoID
@@ -47,9 +61,14 @@ func FindIssueDevLinksByIssue(ctx context.Context, issue *issues_model.Issue) (i
 			if err != nil {
 				return nil, err
 			}
+			pull.BaseRepo = issue.Repo
+			pull.HeadRepo = link.LinkedRepo
+			if err := pull.LoadIssue(ctx); err != nil {
+				return nil, err
+			}
+			pull.Issue.Repo = issue.Repo
 			link.PullRequest = pull
-			link.PullRequest.Issue = issue
-			link.PullRequest.BaseRepo = issue.Repo
+			branchPRExists.Add(fmt.Sprintf("%d-%s", link.LinkedRepoID, pull.HeadBranch))
 		case issues_model.IssueDevLinkTypeBranch:
 			branch, err := git_model.GetBranch(ctx, link.LinkedRepoID, link.LinkIndex)
 			if err != nil {
@@ -57,6 +76,7 @@ func FindIssueDevLinksByIssue(ctx context.Context, issue *issues_model.Issue) (i
 			}
 			link.Branch = branch
 			link.Branch.Repo = link.LinkedRepo
+			link.DisplayBranch = !branchPRExists.Contains(fmt.Sprintf("%d-%s", link.LinkedRepoID, link.LinkIndex))
 		}
 	}
 
diff --git a/services/pull/pull.go b/services/pull/pull.go
index fd316fcb19..4f68a3084c 100644
--- a/services/pull/pull.go
+++ b/services/pull/pull.go
@@ -128,6 +128,31 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *iss
 			return err
 		}
 
+		if !pr.IsWorkInProgress(ctx) {
+			reviewNotifiers, err = issue_service.PullRequestCodeOwnersReview(ctx, issue, pr)
+			if err != nil {
+				return err
+			}
+		}
+
+		if pr.Flow == issues_model.PullRequestFlowGithub {
+			devLinks, err := issues_model.FindDevLinksByBranch(ctx, issue.RepoID, pr.HeadRepoID, pr.HeadBranch)
+			if err != nil {
+				return err
+			}
+			for _, link := range devLinks {
+				if err := issues_model.CreateIssueDevLink(ctx, &issues_model.IssueDevLink{
+					IssueID:      link.IssueID,
+					LinkType:     issues_model.IssueDevLinkTypePullRequest,
+					LinkedRepoID: pr.HeadRepoID,
+					LinkIndex:    strconv.FormatInt(pr.ID, 10),
+				}); err != nil {
+					return err
+				}
+			}
+		}
+
+		// leave creating comment last
 		compareInfo, err := baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(),
 			git.BranchPrefix+pr.BaseBranch, pr.GetGitRefName(), false, false)
 		if err != nil {
@@ -161,30 +186,6 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *iss
 			return err
 		}
 
-		if !pr.IsWorkInProgress(ctx) {
-			reviewNotifiers, err = issue_service.PullRequestCodeOwnersReview(ctx, issue, pr)
-			if err != nil {
-				return err
-			}
-		}
-
-		if pr.Flow == issues_model.PullRequestFlowGithub {
-			devLinks, err := issues_model.FindDevLinksByBranch(ctx, issue.RepoID, pr.HeadRepoID, pr.HeadBranch)
-			if err != nil {
-				return err
-			}
-			for _, link := range devLinks {
-				if err := issues_model.CreateIssueDevLink(ctx, &issues_model.IssueDevLink{
-					IssueID:      link.IssueID,
-					LinkType:     issues_model.IssueDevLinkTypePullRequest,
-					LinkedRepoID: pr.HeadRepoID,
-					LinkIndex:    strconv.FormatInt(pr.ID, 10),
-				}); err != nil {
-					return err
-				}
-			}
-		}
-
 		return nil
 	}); err != nil {
 		// cleanup: this will only remove the reference, the real commit will be clean up when next GC
diff --git a/templates/repo/issue/view_content/sidebar.tmpl b/templates/repo/issue/view_content/sidebar.tmpl
index a37b055501..b27e066238 100644
--- a/templates/repo/issue/view_content/sidebar.tmpl
+++ b/templates/repo/issue/view_content/sidebar.tmpl
@@ -253,9 +253,10 @@
 		</div>
 	</div>
 
-	<div class="divider"></div>
-
-	{{template "repo/issue/view_content/sidebar_development" .}}
+	{{if not .Issue.IsPull}}
+		<div class="divider"></div>
+		{{template "repo/issue/view_content/sidebar_development" .}}
+	{{end}}
 
 	<div class="divider"></div>
 
diff --git a/templates/repo/issue/view_content/sidebar_development.tmpl b/templates/repo/issue/view_content/sidebar_development.tmpl
index ac4045fd3f..f50966ad4a 100644
--- a/templates/repo/issue/view_content/sidebar_development.tmpl
+++ b/templates/repo/issue/view_content/sidebar_development.tmpl
@@ -5,10 +5,13 @@
 {{end}}
 {{range .DevLinks}}
 	{{if .PullRequest}}
+		<span>{{template "shared/issueicon" .PullRequest.Issue}}
 		<a href="{{.PullRequest.Issue.Link}}" class="item">
 			{{.PullRequest.Issue.Title}}
 		</a>
-		Created {{.PullRequest.Issue.CreatedAt}}
+		</span>
+		<div>
+		Created {{DateTime "short" .PullRequest.Issue.CreatedUnix}}
 		{{if .PullRequest.HasMerged}}
 			Completed
 			{{.PullRequest.MergedCommitID}}
@@ -16,15 +19,16 @@
 		{{else if .PullRequest.ChangedProtectedFiles}}
 			Merge conflicts
 		{{end}}
-	{{else if .Branch}}
+		</div>
+	{{else if and .Branch .DisplayBranch}}
 		<span>
 			{{svg "octicon-git-branch" 14}}
-			<a href="{{.Branch}}" class="item">
+			<a href="{{.Branch.Repo.Link}}/src/branch/{{.Branch.Name}}" class="item">
 				<span class="gt-ellipsis">{{.Branch.Name}}</span>
 			</a>
 		</span>
 		<div>Latest commit {{DateTime "short" .Branch.CommitTime}}</div>
-		<a href="{{$.Issue.Repo.Link}}/compare/main...{{.Branch.Name}}">{{ctx.Locale.Tr "repo.pulls.new"}}</a>
+		<a href="{{$.Issue.Repo.Link}}/compare/{{$.Issue.Repo.DefaultBranch}}...{{.Branch.Repo.FullName}}:{{.Branch.Name}}">{{ctx.Locale.Tr "repo.pulls.new"}}</a>
 	{{end}}
 {{end}}
 </div>
@@ -45,7 +49,7 @@
 			<div class="field">
 				<label for="source_repository">{{ctx.Locale.Tr "repository"}}</label>
 				<div class="ui fluid dropdown selection">
-					<select name="source_repository">
+					<select name="repo_id">
 						<option value=""> </option>
 						{{range .AllowedRepos}}
 							<option value="{{.ID}}">{{.FullName}}</option>
@@ -86,7 +90,7 @@
 
 			<div class="text right actions">
 				<button class="ui cancel button">{{ctx.Locale.Tr "settings.cancel"}}</button>
-				<button class="ui red button">{{ctx.Locale.Tr "repo.branch.new_branch"}}</button>
+				<button class="ui primary button">{{ctx.Locale.Tr "repo.branch.new_branch"}}</button>
 			</div>
 		</form>
 	</div>
diff --git a/templates/repo/issue/view_title.tmpl b/templates/repo/issue/view_title.tmpl
index 1243681f3a..42bb421201 100644
--- a/templates/repo/issue/view_title.tmpl
+++ b/templates/repo/issue/view_title.tmpl
@@ -118,6 +118,10 @@
 					·
 					{{ctx.Locale.TrN .Issue.NumComments "repo.issues.num_comments_1" "repo.issues.num_comments" .Issue.NumComments}}
 				</span>
+				{{if .MaybeFixed}}
+					{{$fixedStr:= printf "<a href=\"%s\">#%d</a>" .MaybeFixed.Issue.Link .MaybeFixed.Index}}
+					· <span>{{ctx.Locale.Tr "repo.issues.maybefixed" ($fixedStr|SafeHTML)}}</span>
+				{{end}}
 			{{end}}
 		</div>
 	</div>