From 66a9ef90362db1280808ff74e40c2e670f992fcb Mon Sep 17 00:00:00 2001 From: Sijmen Schoon Date: Fri, 15 May 2020 00:55:43 +0200 Subject: [PATCH] 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 Co-authored-by: techknowlogick --- models/migrations/migrations.go | 2 ++ models/migrations/v139.go | 25 +++++++++++++++ modules/git/utils.go | 13 ++++++++ modules/git/utils_test.go | 31 +++++++++++++++++++ modules/webhook/slack.go | 10 ++---- routers/repo/issue.go | 4 +++ routers/user/home.go | 4 +++ services/issue/issue.go | 16 ++++++++++ services/issue/issue_test.go | 30 ++++++++++++++++++ .../repo/issue/branch_selector_field.tmpl | 6 ++-- templates/repo/issue/list.tmpl | 4 +-- templates/repo/issue/milestone_issues.tmpl | 4 +-- templates/user/dashboard/issues.tmpl | 4 +-- web_src/js/index.js | 7 ++--- 14 files changed, 139 insertions(+), 21 deletions(-) create mode 100644 models/migrations/v139.go create mode 100644 modules/git/utils_test.go create mode 100644 services/issue/issue_test.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 6868aad7b..00d84da2e 100644 --- a/models/migrations/migrations.go +++ b/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 diff --git a/models/migrations/v139.go b/models/migrations/v139.go new file mode 100644 index 000000000..ffacad0a0 --- /dev/null +++ b/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 +} diff --git a/modules/git/utils.go b/modules/git/utils.go index 2b823366b..83209924c 100644 --- a/modules/git/utils.go +++ b/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) { diff --git a/modules/git/utils_test.go b/modules/git/utils_test.go new file mode 100644 index 000000000..9a2d481b6 --- /dev/null +++ b/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")) +} diff --git a/modules/webhook/slack.go b/modules/webhook/slack.go index e3715ab00..1e9413efd 100644 --- a/modules/webhook/slack.go +++ b/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) { diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 6a713f906..afe64c731 100644 --- a/routers/repo/issue.go +++ b/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) } diff --git a/routers/user/home.go b/routers/user/home.go index 199694f23..2fc0c60aa 100644 --- a/routers/user/home.go +++ b/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] diff --git a/services/issue/issue.go b/services/issue/issue.go index aa06ba409..64d69119b 100644 --- a/services/issue/issue.go +++ b/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 +} diff --git a/services/issue/issue_test.go b/services/issue/issue_test.go new file mode 100644 index 000000000..caae77361 --- /dev/null +++ b/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) +} diff --git a/templates/repo/issue/branch_selector_field.tmpl b/templates/repo/issue/branch_selector_field.tmpl index a584152d6..4f80c13e5 100644 --- a/templates/repo/issue/branch_selector_field.tmpl +++ b/templates/repo/issue/branch_selector_field.tmpl @@ -2,7 +2,7 @@
- {{if .Issue.Ref}}{{.Issue.Ref}}{{else}}{{.i18n.Tr "repo.issues.no_ref"}}{{end}} + {{if .Issue.Ref}}{{$.RefEndName}}{{else}}{{.i18n.Tr "repo.issues.no_ref"}}{{end}}
@@ -28,12 +28,12 @@
diff --git a/templates/repo/issue/list.tmpl b/templates/repo/issue/list.tmpl index 2b9078dfc..f64de74e8 100644 --- a/templates/repo/issue/list.tmpl +++ b/templates/repo/issue/list.tmpl @@ -247,8 +247,8 @@ {{end}} {{if .Ref}} - - {{svg "octicon-git-branch" 16}} {{.Ref}} + + {{svg "octicon-git-branch" 16}} {{index $.IssueRefEndNames .ID}} {{end}} {{$tasks := .GetTasks}} diff --git a/templates/repo/issue/milestone_issues.tmpl b/templates/repo/issue/milestone_issues.tmpl index 665d26226..682469a14 100644 --- a/templates/repo/issue/milestone_issues.tmpl +++ b/templates/repo/issue/milestone_issues.tmpl @@ -218,8 +218,8 @@ {{end}} {{if .Ref}} - - {{svg "octicon-git-branch" 16}} {{.Ref}} + + {{svg "octicon-git-branch" 16}} {{index $.IssueRefEndNames .ID}} {{end}} {{$tasks := .GetTasks}} diff --git a/templates/user/dashboard/issues.tmpl b/templates/user/dashboard/issues.tmpl index f9bf2c157..ebe552f50 100644 --- a/templates/user/dashboard/issues.tmpl +++ b/templates/user/dashboard/issues.tmpl @@ -149,8 +149,8 @@ {{end}} {{if .Ref}} - - {{svg "octicon-git-branch" 16}} {{.Ref}} + + {{svg "octicon-git-branch" 16}} {{index $.IssueRefEndNames .ID}} {{end}} {{range .Assignees}} diff --git a/web_src/js/index.js b/web_src/js/index.js index c7b893487..91d16192d 100644 --- a/web_src/js/index.js +++ b/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');