Fix ref links in issue overviews for tags (#8742)

* Properly generate ref URLs

Tags used to not generate correct URLs (src/branch/tags/1.0.0 instead of
src/tags/1.0.0).

Also cleans up some code around it with the created helper functions.

* Fix formatting and create migration

* Add copyright head to utils_test

* Use a raw query for the ref migration

* Remove semicolon

* Quote column and table names in migration SQL

* Change || to CONCAT, since MSSQL does not support ||

* Make migration engine aware

* Add missing import

* Move ref EndName and URL to the issue service

* Fix tests

* Add test for commit refs

* Update issue.go

* Use the right command for building JavaScript bundles

* Prepare for merge

* Check for refs/* before prepending in migration

* Update services/issue/issue_test.go

* Update modules/git/utils_test.go

Co-authored-by: techknowlogick <techknowlogick@gitea.io>
Co-authored-by: techknowlogick <matti@mdranta.net>

models/migrations/migrations.go

@@ -210,6 +210,8 @@ var migrations = []Migration{
 	NewMigration("Add Branch Protection Block Outdated Branch", addBlockOnOutdatedBranch),
 	// v138 -> v139
 	NewMigration("Add ResolveDoerID to Comment table", addResolveDoerIDCommentColumn),
+	// v139 -> v140
+	NewMigration("prepend refs/heads/ to issue refs", prependRefsHeadsToIssueRefs),
 }
 
 // GetCurrentDBVersion returns the current db version

models/migrations/v139.go

@@ -0,0 +1,25 @@
+// Copyright 2019 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package migrations
+
+import (
+	"code.gitea.io/gitea/modules/setting"
+
+	"xorm.io/xorm"
+)
+
+func prependRefsHeadsToIssueRefs(x *xorm.Engine) error {
+	var query string
+
+	switch {
+	case setting.Database.UseMSSQL:
+		query = "UPDATE `issue` SET `ref` = 'refs/heads/' + `ref` WHERE `ref` IS NOT NULL AND `ref` <> '' AND `ref` NOT LIKE 'refs/%'"
+	default:
+		query = "UPDATE `issue` SET `ref` = 'refs/heads/' || `ref` WHERE `ref` IS NOT NULL AND `ref` <> '' AND `ref` NOT LIKE 'refs/%'"
+	}
+
+	_, err := x.Exec(query)
+	return err
+}

modules/git/utils.go

@@ -88,6 +88,19 @@ func RefEndName(refStr string) string {
 	return refStr
 }
 
+// RefURL returns the absolute URL for a ref in a repository
+func RefURL(repoURL, ref string) string {
+	refName := RefEndName(ref)
+	switch {
+	case strings.HasPrefix(ref, BranchPrefix):
+		return repoURL + "/src/branch/" + refName
+	case strings.HasPrefix(ref, TagPrefix):
+		return repoURL + "/src/tag/" + refName
+	default:
+		return repoURL + "/src/commit/" + refName
+	}
+}
+
 // SplitRefName splits a full refname to reftype and simple refname
 func SplitRefName(refStr string) (string, string) {
 	if strings.HasPrefix(refStr, BranchPrefix) {

modules/git/utils_test.go

@@ -0,0 +1,31 @@
+// Copyright 2020 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package git
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestRefEndName(t *testing.T) {
+	// Test branch names (with and without slash).
+	assert.Equal(t, "foo", RefEndName("refs/heads/foo"))
+	assert.Equal(t, "feature/foo", RefEndName("refs/heads/feature/foo"))
+
+	// Test tag names (with and without slash).
+	assert.Equal(t, "foo", RefEndName("refs/tags/foo"))
+	assert.Equal(t, "release/foo", RefEndName("refs/tags/release/foo"))
+
+	// Test commit hashes.
+	assert.Equal(t, "c0ffee", RefEndName("c0ffee"))
+}
+
+func TestRefURL(t *testing.T) {
+	repoURL := "/user/repo"
+	assert.Equal(t, repoURL+"/src/branch/foo", RefURL(repoURL, "refs/heads/foo"))
+	assert.Equal(t, repoURL+"/src/tag/foo", RefURL(repoURL, "refs/tags/foo"))
+	assert.Equal(t, repoURL+"/src/commit/c0ffee", RefURL(repoURL, "c0ffee"))
+}

modules/webhook/slack.go

@@ -93,15 +93,9 @@ func SlackLinkFormatter(url string, text string) string {
 
 // SlackLinkToRef slack-formatter link to a repo ref
 func SlackLinkToRef(repoURL, ref string) string {
+	url := git.RefURL(repoURL, ref)
 	refName := git.RefEndName(ref)
-	switch {
-	case strings.HasPrefix(ref, git.BranchPrefix):
-		return SlackLinkFormatter(repoURL+"/src/branch/"+refName, refName)
-	case strings.HasPrefix(ref, git.TagPrefix):
-		return SlackLinkFormatter(repoURL+"/src/tag/"+refName, refName)
-	default:
-		return SlackLinkFormatter(repoURL+"/src/commit/"+refName, refName)
-	}
+	return SlackLinkFormatter(url, refName)
 }
 
 func getSlackCreatePayload(p *api.CreatePayload, slack *SlackMeta) (*SlackPayload, error) {

routers/repo/issue.go

@@ -286,6 +286,9 @@ func issues(ctx *context.Context, milestoneID int64, isPullOption util.OptionalB
 		assigneeID = 0 // Reset ID to prevent unexpected selection of assignee.
 	}
 
+	ctx.Data["IssueRefEndNames"], ctx.Data["IssueRefURLs"] =
+		issue_service.GetRefEndNamesAndURLs(issues, ctx.Repo.RepoLink)
+
 	ctx.Data["ApprovalCounts"] = func(issueID int64, typ string) int64 {
 		counts, ok := approvalCounts[issueID]
 		if !ok || len(counts) == 0 {
@@ -1127,6 +1130,7 @@ func ViewIssue(ctx *context.Context) {
 	ctx.Data["HasIssuesOrPullsWritePermission"] = ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull)
 	ctx.Data["IsRepoAdmin"] = ctx.IsSigned && (ctx.Repo.IsAdmin() || ctx.User.IsAdmin)
 	ctx.Data["LockReasons"] = setting.Repository.Issue.LockReasons
+	ctx.Data["RefEndName"] = git.RefEndName(issue.Ref)
 	ctx.HTML(200, tplIssueView)
 }
 

routers/user/home.go

@@ -22,6 +22,7 @@ import (
 	"code.gitea.io/gitea/modules/markup/markdown"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
+	issue_service "code.gitea.io/gitea/services/issue"
 	pull_service "code.gitea.io/gitea/services/pull"
 
 	"github.com/keybase/go-crypto/openpgp"
@@ -624,6 +625,9 @@ func Issues(ctx *context.Context) {
 		totalIssues = int(allIssueStats.ClosedCount)
 	}
 
+	ctx.Data["IssueRefEndNames"], ctx.Data["IssueRefURLs"] =
+		issue_service.GetRefEndNamesAndURLs(issues, ctx.Query("RepoLink"))
+
 	ctx.Data["Issues"] = issues
 	ctx.Data["ApprovalCounts"] = func(issueID int64, typ string) int64 {
 		counts, ok := approvalCounts[issueID]

services/issue/issue.go

@@ -6,7 +6,9 @@ package issue
 
 import (
 	"code.gitea.io/gitea/models"
+	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/notification"
+	"code.gitea.io/gitea/modules/util"
 )
 
 // NewIssue creates new issue with labels for repository.
@@ -128,3 +130,17 @@ func AddAssigneeIfNotAssigned(issue *models.Issue, doer *models.User, assigneeID
 
 	return nil
 }
+
+// GetRefEndNamesAndURLs retrieves the ref end names (e.g. refs/heads/branch-name -> branch-name)
+// and their respective URLs.
+func GetRefEndNamesAndURLs(issues []*models.Issue, repoLink string) (map[int64]string, map[int64]string) {
+	var issueRefEndNames = make(map[int64]string, len(issues))
+	var issueRefURLs = make(map[int64]string, len(issues))
+	for _, issue := range issues {
+		if issue.Ref != "" {
+			issueRefEndNames[issue.ID] = git.RefEndName(issue.Ref)
+			issueRefURLs[issue.ID] = git.RefURL(repoLink, util.PathEscapeSegments(issue.Ref))
+		}
+	}
+	return issueRefEndNames, issueRefURLs
+}

services/issue/issue_test.go

@@ -0,0 +1,30 @@
+// Copyright 2020 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package issue
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/models"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetRefEndNamesAndURLs(t *testing.T) {
+	issues := []*models.Issue{
+		{ID: 1, Ref: "refs/heads/branch1"},
+		{ID: 2, Ref: "refs/tags/tag1"},
+		{ID: 3, Ref: "c0ffee"},
+	}
+	repoLink := "/foo/bar"
+
+	endNames, urls := GetRefEndNamesAndURLs(issues, repoLink)
+	assert.EqualValues(t, map[int64]string{1: "branch1", 2: "tag1", 3: "c0ffee"}, endNames)
+	assert.EqualValues(t, map[int64]string{
+		1: repoLink + "/src/branch/branch1",
+		2: repoLink + "/src/tag/tag1",
+		3: repoLink + "/src/commit/c0ffee",
+	}, urls)
+}

templates/repo/issue/branch_selector_field.tmpl

@@ -2,7 +2,7 @@
 <input id="ref_selector" name="ref" type="hidden" value="{{.Issue.Ref}}">
 <div class="ui {{if .ReadOnly}}disabled{{end}} floating filter select-branch dropdown" data-no-results="{{.i18n.Tr "repo.pulls.no_results"}}">
 	<div class="ui basic small button">
-		<span class="text branch-name">{{if .Issue.Ref}}{{.Issue.Ref}}{{else}}{{.i18n.Tr "repo.issues.no_ref"}}{{end}}</span>
+		<span class="text branch-name">{{if .Issue.Ref}}{{$.RefEndName}}{{else}}{{.i18n.Tr "repo.issues.no_ref"}}{{end}}</span>
 		<i class="dropdown icon"></i>
 	</div>
 	<div class="menu">
@@ -28,12 +28,12 @@
 		</div>
 		<div id="branch-list" class="scrolling menu reference-list-menu">
 		{{range .Branches}}
-			<div class="item" data-id="{{.}}" data-id-selector="#ref_selector">{{.}}</div>
+			<div class="item" data-id="refs/heads/{{.}}" data-name="{{.}}" data-id-selector="#ref_selector">{{.}}</div>
 		{{end}}
 		</div>
 		<div id="tag-list" class="scrolling menu reference-list-menu" style="display: none">
 		{{range .Tags}}
-			<div class="item" data-id="{{.}}" data-id-selector="#ref_selector">{{.}}</div>
+			<div class="item" data-id="refs/tags/{{.}}" data-name="tags/{{.}}" data-id-selector="#ref_selector">{{.}}</div>
 		{{end}}
 		</div>
 	</div>

templates/repo/issue/list.tmpl

@@ -247,8 +247,8 @@
 							</a>
 						{{end}}
 						{{if .Ref}}
-							<a class="ref" href="{{$.RepoLink}}/src/branch/{{.Ref | PathEscapeSegments}}">
-								{{svg "octicon-git-branch" 16}} {{.Ref}}
+							<a class="ref" href="{{index $.IssueRefURLs .ID}}">
+								{{svg "octicon-git-branch" 16}} {{index $.IssueRefEndNames .ID}}
 							</a>
 						{{end}}
 						{{$tasks := .GetTasks}}

templates/repo/issue/milestone_issues.tmpl

@@ -218,8 +218,8 @@
 						{{end}}
 
 						{{if .Ref}}
-							<a class="ref" href="{{$.RepoLink}}/src/branch/{{.Ref}}">
-								{{svg "octicon-git-branch" 16}} {{.Ref}}
+							<a class="ref" href="{{index $.IssueRefURLs .ID}}">
+								{{svg "octicon-git-branch" 16}} {{index $.IssueRefEndNames .ID}}
 							</a>
 						{{end}}
 						{{$tasks := .GetTasks}}

templates/user/dashboard/issues.tmpl

@@ -149,8 +149,8 @@
 									</a>
 								{{end}}
 								{{if .Ref}}
-									<a class="ref" href="{{AppSubUrl}}/{{.Repo.Owner.Name}}/{{.Repo.Name}}/src/branch/{{.Ref}}">
-										{{svg "octicon-git-branch" 16}} {{.Ref}}
+									<a class="ref" href="{{AppSubUrl}}/{{.Repo.Owner.Name}}/{{.Repo.Name}}{{index $.IssueRefURLs .ID}}">
+										{{svg "octicon-git-branch" 16}} {{index $.IssueRefEndNames .ID}}
 									</a>
 								{{end}}
 								{{range .Assignees}}

web_src/js/index.js

@@ -114,10 +114,9 @@ function initEditForm() {
 function initBranchSelector() {
   const $selectBranch = $('.ui.select-branch');
   const $branchMenu = $selectBranch.find('.reference-list-menu');
-  $branchMenu.find('.item:not(.no-select)').on('click', function () {
-    const selectedValue = $(this).data('id');
-    $($(this).data('id-selector')).val(selectedValue);
-    $selectBranch.find('.ui .branch-name').text(selectedValue);
+  $branchMenu.find('.item:not(.no-select)').click(function () {
+    $($(this).data('id-selector')).val($(this).data('id'));
+    $selectBranch.find('.ui .branch-name').text($(this).data('name'));
   });
   $selectBranch.find('.reference.column').on('click', function () {
     $selectBranch.find('.scrolling.reference-list-menu').css('display', 'none');