Skip to content

Commit 513375c

Browse files
ethantkoeniglunny
ethantkoenig
authored and
lunny
committed
Make URL scheme unambiguous (#2408)
* Make URL scheme unambiguous Redirect old routes to new routes * Fix redirects to new URL scheme, and update template * Fix branches/_new endpoints, and update integration test
1 parent 6e98812 commit 513375c

File tree

20 files changed

+279
-144
lines changed

20 files changed

+279
-144
lines changed

‎integrations/editor_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ func testEditFile(t *testing.T, session *TestSession, user, repo, branch, filePa
111111
resp = session.MakeRequest(t, req, http.StatusFound)
112112

113113
// Verify the change
114-
req = NewRequest(t, "GET", path.Join(user, repo, "raw", branch, filePath))
114+
req = NewRequest(t, "GET", path.Join(user, repo, "raw/branch", branch, filePath))
115115
resp = session.MakeRequest(t, req, http.StatusOK)
116116
assert.EqualValues(t, newContent, string(resp.Body))
117117

@@ -142,7 +142,7 @@ func testEditFileToNewBranch(t *testing.T, session *TestSession, user, repo, bra
142142
resp = session.MakeRequest(t, req, http.StatusFound)
143143

144144
// Verify the change
145-
req = NewRequest(t, "GET", path.Join(user, repo, "raw", targetBranch, filePath))
145+
req = NewRequest(t, "GET", path.Join(user, repo, "raw/branch", targetBranch, filePath))
146146
resp = session.MakeRequest(t, req, http.StatusOK)
147147
assert.EqualValues(t, newContent, string(resp.Body))
148148

‎integrations/links_test.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import (
1010
"testing"
1111

1212
api "code.gitea.io/sdk/gitea"
13+
14+
"github.com/stretchr/testify/assert"
1315
)
1416

1517
func TestLinksNoLogin(t *testing.T) {
@@ -38,6 +40,20 @@ func TestLinksNoLogin(t *testing.T) {
3840
}
3941
}
4042

43+
func TestRedirectsNoLogin(t *testing.T) {
44+
prepareTestEnv(t)
45+
46+
var redirects = map[string]string{
47+
"/user2/repo1/commits/master": "/user2/repo1/commits/branch/master",
48+
"/user2/repo1/src/master": "/user2/repo1/src/branch/master",
49+
}
50+
for link, redirectLink := range redirects {
51+
req := NewRequest(t, "GET", link)
52+
resp := MakeRequest(t, req, http.StatusFound)
53+
assert.EqualValues(t, redirectLink, RedirectURL(t, resp))
54+
}
55+
}
56+
4157
func testLinksAsUser(userName string, t *testing.T) {
4258
var links = []string{
4359
"/explore/repos",
@@ -99,7 +115,7 @@ func testLinksAsUser(userName string, t *testing.T) {
99115
"",
100116
"/issues",
101117
"/pulls",
102-
"/commits/master",
118+
"/commits/branch/master",
103119
"/graph",
104120
"/settings",
105121
"/settings/collaboration",

‎integrations/repo_branch_test.go

Lines changed: 52 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,14 @@ import (
1414
"github.com/stretchr/testify/assert"
1515
)
1616

17-
func testCreateBranch(t *testing.T, session *TestSession, user, repo, oldRefName, newBranchName string, expectedStatus int) string {
17+
func testCreateBranch(t *testing.T, session *TestSession, user, repo, oldRefSubURL, newBranchName string, expectedStatus int) string {
1818
var csrf string
1919
if expectedStatus == http.StatusNotFound {
20-
csrf = GetCSRF(t, session, path.Join(user, repo, "src/master"))
20+
csrf = GetCSRF(t, session, path.Join(user, repo, "src/branch/master"))
2121
} else {
22-
csrf = GetCSRF(t, session, path.Join(user, repo, "src", oldRefName))
22+
csrf = GetCSRF(t, session, path.Join(user, repo, "src", oldRefSubURL))
2323
}
24-
req := NewRequestWithValues(t, "POST", path.Join(user, repo, "branches/_new", oldRefName), map[string]string{
24+
req := NewRequestWithValues(t, "POST", path.Join(user, repo, "branches/_new", oldRefSubURL), map[string]string{
2525
"_csrf": csrf,
2626
"new_branch_name": newBranchName,
2727
})
@@ -34,72 +34,72 @@ func testCreateBranch(t *testing.T, session *TestSession, user, repo, oldRefName
3434

3535
func TestCreateBranch(t *testing.T) {
3636
tests := []struct {
37-
OldBranchOrCommit string
38-
NewBranch string
39-
CreateRelease string
40-
FlashMessage string
41-
ExpectedStatus int
37+
OldRefSubURL string
38+
NewBranch string
39+
CreateRelease string
40+
FlashMessage string
41+
ExpectedStatus int
4242
}{
4343
{
44-
OldBranchOrCommit: "master",
45-
NewBranch: "feature/test1",
46-
ExpectedStatus: http.StatusFound,
47-
FlashMessage: i18n.Tr("en", "repo.branch.create_success", "feature/test1"),
44+
OldRefSubURL: "branch/master",
45+
NewBranch: "feature/test1",
46+
ExpectedStatus: http.StatusFound,
47+
FlashMessage: i18n.Tr("en", "repo.branch.create_success", "feature/test1"),
4848
},
4949
{
50-
OldBranchOrCommit: "master",
51-
NewBranch: "",
52-
ExpectedStatus: http.StatusFound,
53-
FlashMessage: i18n.Tr("en", "form.NewBranchName") + i18n.Tr("en", "form.require_error"),
50+
OldRefSubURL: "branch/master",
51+
NewBranch: "",
52+
ExpectedStatus: http.StatusFound,
53+
FlashMessage: i18n.Tr("en", "form.NewBranchName") + i18n.Tr("en", "form.require_error"),
5454
},
5555
{
56-
OldBranchOrCommit: "master",
57-
NewBranch: "feature=test1",
58-
ExpectedStatus: http.StatusFound,
59-
FlashMessage: i18n.Tr("en", "form.NewBranchName") + i18n.Tr("en", "form.git_ref_name_error"),
56+
OldRefSubURL: "branch/master",
57+
NewBranch: "feature=test1",
58+
ExpectedStatus: http.StatusFound,
59+
FlashMessage: i18n.Tr("en", "form.NewBranchName") + i18n.Tr("en", "form.git_ref_name_error"),
6060
},
6161
{
62-
OldBranchOrCommit: "master",
63-
NewBranch: strings.Repeat("b", 101),
64-
ExpectedStatus: http.StatusFound,
65-
FlashMessage: i18n.Tr("en", "form.NewBranchName") + i18n.Tr("en", "form.max_size_error", "100"),
62+
OldRefSubURL: "branch/master",
63+
NewBranch: strings.Repeat("b", 101),
64+
ExpectedStatus: http.StatusFound,
65+
FlashMessage: i18n.Tr("en", "form.NewBranchName") + i18n.Tr("en", "form.max_size_error", "100"),
6666
},
6767
{
68-
OldBranchOrCommit: "master",
69-
NewBranch: "master",
70-
ExpectedStatus: http.StatusFound,
71-
FlashMessage: i18n.Tr("en", "repo.branch.branch_already_exists", "master"),
68+
OldRefSubURL: "branch/master",
69+
NewBranch: "master",
70+
ExpectedStatus: http.StatusFound,
71+
FlashMessage: i18n.Tr("en", "repo.branch.branch_already_exists", "master"),
7272
},
7373
{
74-
OldBranchOrCommit: "master",
75-
NewBranch: "master/test",
76-
ExpectedStatus: http.StatusFound,
77-
FlashMessage: i18n.Tr("en", "repo.branch.branch_name_conflict", "master/test", "master"),
74+
OldRefSubURL: "branch/master",
75+
NewBranch: "master/test",
76+
ExpectedStatus: http.StatusFound,
77+
FlashMessage: i18n.Tr("en", "repo.branch.branch_name_conflict", "master/test", "master"),
7878
},
7979
{
80-
OldBranchOrCommit: "acd1d892867872cb47f3993468605b8aa59aa2e0",
81-
NewBranch: "feature/test2",
82-
ExpectedStatus: http.StatusNotFound,
80+
OldRefSubURL: "commit/acd1d892867872cb47f3993468605b8aa59aa2e0",
81+
NewBranch: "feature/test2",
82+
ExpectedStatus: http.StatusNotFound,
8383
},
8484
{
85-
OldBranchOrCommit: "65f1bf27bc3bf70f64657658635e66094edbcb4d",
86-
NewBranch: "feature/test3",
87-
ExpectedStatus: http.StatusFound,
88-
FlashMessage: i18n.Tr("en", "repo.branch.create_success", "feature/test3"),
85+
OldRefSubURL: "commit/65f1bf27bc3bf70f64657658635e66094edbcb4d",
86+
NewBranch: "feature/test3",
87+
ExpectedStatus: http.StatusFound,
88+
FlashMessage: i18n.Tr("en", "repo.branch.create_success", "feature/test3"),
8989
},
9090
{
91-
OldBranchOrCommit: "master",
92-
NewBranch: "v1.0.0",
93-
CreateRelease: "v1.0.0",
94-
ExpectedStatus: http.StatusFound,
95-
FlashMessage: i18n.Tr("en", "repo.branch.tag_collision", "v1.0.0"),
91+
OldRefSubURL: "branch/master",
92+
NewBranch: "v1.0.0",
93+
CreateRelease: "v1.0.0",
94+
ExpectedStatus: http.StatusFound,
95+
FlashMessage: i18n.Tr("en", "repo.branch.tag_collision", "v1.0.0"),
9696
},
9797
{
98-
OldBranchOrCommit: "v1.0.0",
99-
NewBranch: "feature/test4",
100-
CreateRelease: "v1.0.0",
101-
ExpectedStatus: http.StatusFound,
102-
FlashMessage: i18n.Tr("en", "repo.branch.create_success", "feature/test4"),
98+
OldRefSubURL: "tag/v1.0.0",
99+
NewBranch: "feature/test4",
100+
CreateRelease: "v1.0.0",
101+
ExpectedStatus: http.StatusFound,
102+
FlashMessage: i18n.Tr("en", "repo.branch.create_success", "feature/test4"),
103103
},
104104
}
105105
for _, test := range tests {
@@ -108,7 +108,7 @@ func TestCreateBranch(t *testing.T) {
108108
if test.CreateRelease != "" {
109109
createNewRelease(t, session, "/user2/repo1", test.CreateRelease, test.CreateRelease, false, false)
110110
}
111-
redirectURL := testCreateBranch(t, session, "user2", "repo1", test.OldBranchOrCommit, test.NewBranch, test.ExpectedStatus)
111+
redirectURL := testCreateBranch(t, session, "user2", "repo1", test.OldRefSubURL, test.NewBranch, test.ExpectedStatus)
112112
if test.ExpectedStatus == http.StatusFound {
113113
req := NewRequest(t, "GET", redirectURL)
114114
resp := session.MakeRequest(t, req, http.StatusOK)
@@ -124,7 +124,7 @@ func TestCreateBranch(t *testing.T) {
124124
func TestCreateBranchInvalidCSRF(t *testing.T) {
125125
prepareTestEnv(t)
126126
session := loginUser(t, "user2")
127-
req := NewRequestWithValues(t, "POST", "user2/repo1/branches/_new/master", map[string]string{
127+
req := NewRequestWithValues(t, "POST", "user2/repo1/branches/_new/branch/master", map[string]string{
128128
"_csrf": "fake_csrf",
129129
"new_branch_name": "test",
130130
})

‎integrations/repo_commits_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ func TestRepoCommits(t *testing.T) {
2020
session := loginUser(t, "user2")
2121

2222
// Request repository commits page
23-
req := NewRequest(t, "GET", "/user2/repo1/commits/master")
23+
req := NewRequest(t, "GET", "/user2/repo1/commits/branch/master")
2424
resp := session.MakeRequest(t, req, http.StatusOK)
2525

2626
doc := NewHTMLParser(t, resp.Body)
@@ -35,7 +35,7 @@ func doTestRepoCommitWithStatus(t *testing.T, state string, classes ...string) {
3535
session := loginUser(t, "user2")
3636

3737
// Request repository commits page
38-
req := NewRequest(t, "GET", "/user2/repo1/commits/master")
38+
req := NewRequest(t, "GET", "/user2/repo1/commits/branch/master")
3939
resp := session.MakeRequest(t, req, http.StatusOK)
4040

4141
doc := NewHTMLParser(t, resp.Body)
@@ -56,7 +56,7 @@ func doTestRepoCommitWithStatus(t *testing.T, state string, classes ...string) {
5656

5757
resp = session.MakeRequest(t, req, http.StatusCreated)
5858

59-
req = NewRequest(t, "GET", "/user2/repo1/commits/master")
59+
req = NewRequest(t, "GET", "/user2/repo1/commits/branch/master")
6060
resp = session.MakeRequest(t, req, http.StatusOK)
6161

6262
doc = NewHTMLParser(t, resp.Body)

‎models/webhook_slack.go

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,12 +80,22 @@ func SlackLinkFormatter(url string, text string) string {
8080
return fmt.Sprintf("<%s|%s>", url, SlackTextFormatter(text))
8181
}
8282

83-
func getSlackCreatePayload(p *api.CreatePayload, slack *SlackMeta) (*SlackPayload, error) {
84-
// created tag/branch
85-
refName := git.RefEndName(p.Ref)
83+
// SlackLinkToRef slack-formatter link to a repo ref
84+
func SlackLinkToRef(repoURL, ref string) string {
85+
refName := git.RefEndName(ref)
86+
switch {
87+
case strings.HasPrefix(ref, git.BranchPrefix):
88+
return SlackLinkFormatter(repoURL+"/src/branch/"+refName, refName)
89+
case strings.HasPrefix(ref, git.TagPrefix):
90+
return SlackLinkFormatter(repoURL+"/src/tag/"+refName, refName)
91+
default:
92+
return SlackLinkFormatter(repoURL+"/src/commit/"+refName, refName)
93+
}
94+
}
8695

96+
func getSlackCreatePayload(p *api.CreatePayload, slack *SlackMeta) (*SlackPayload, error) {
8797
repoLink := SlackLinkFormatter(p.Repo.HTMLURL, p.Repo.Name)
88-
refLink := SlackLinkFormatter(p.Repo.HTMLURL+"/src/"+refName, refName)
98+
refLink := SlackLinkToRef(p.Repo.HTMLURL, p.Ref)
8999
text := fmt.Sprintf("[%s:%s] %s created by %s", repoLink, refLink, p.RefType, p.Sender.UserName)
90100

91101
return &SlackPayload{
@@ -99,7 +109,6 @@ func getSlackCreatePayload(p *api.CreatePayload, slack *SlackMeta) (*SlackPayloa
99109
func getSlackPushPayload(p *api.PushPayload, slack *SlackMeta) (*SlackPayload, error) {
100110
// n new commits
101111
var (
102-
branchName = git.RefEndName(p.Ref)
103112
commitDesc string
104113
commitString string
105114
)
@@ -116,7 +125,7 @@ func getSlackPushPayload(p *api.PushPayload, slack *SlackMeta) (*SlackPayload, e
116125
}
117126

118127
repoLink := SlackLinkFormatter(p.Repo.HTMLURL, p.Repo.Name)
119-
branchLink := SlackLinkFormatter(p.Repo.HTMLURL+"/src/"+branchName, branchName)
128+
branchLink := SlackLinkToRef(p.Repo.HTMLURL, p.Ref)
120129
text := fmt.Sprintf("[%s:%s] %s pushed by %s", repoLink, branchLink, commitString, p.Pusher.UserName)
121130

122131
var attachmentText string

0 commit comments

Comments
 (0)