Skip to content

Commit 194479a

Browse files
authored
Use a predictiable fork URL to allow forking repositories without providing a repo ID (#29519)
Close #29512 The "fork" URL: * Before: `/repo/fork/{RepoID}` * After: `/{OwnerName}/{RepoName}/fork`
1 parent 2ca5daf commit 194479a

File tree

5 files changed

+8
-30
lines changed

5 files changed

+8
-30
lines changed

routers/web/repo/pull.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ func getRepository(ctx *context.Context, repoID int64) *repo_model.Repository {
112112
}
113113

114114
func getForkRepository(ctx *context.Context) *repo_model.Repository {
115-
forkRepo := getRepository(ctx, ctx.ParamsInt64(":repoid"))
115+
forkRepo := ctx.Repo.Repository
116116
if ctx.Written() {
117117
return nil
118118
}

routers/web/web.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -956,10 +956,6 @@ func registerRoutes(m *web.Route) {
956956
m.Post("/create", web.Bind(forms.CreateRepoForm{}), repo.CreatePost)
957957
m.Get("/migrate", repo.Migrate)
958958
m.Post("/migrate", web.Bind(forms.MigrateRepoForm{}), repo.MigratePost)
959-
m.Group("/fork", func() {
960-
m.Combo("/{repoid}").Get(repo.Fork).
961-
Post(web.Bind(forms.CreateRepoForm{}), repo.ForkPost)
962-
}, context.RepoIDAssignment(), context.UnitTypes(), reqRepoCodeReader)
963959
m.Get("/search", repo.SearchRepo)
964960
}, reqSignIn)
965961

@@ -1255,6 +1251,8 @@ func registerRoutes(m *web.Route) {
12551251
m.Post("/delete", repo.DeleteBranchPost)
12561252
m.Post("/restore", repo.RestoreBranchPost)
12571253
}, context.RepoMustNotBeArchived(), reqRepoCodeWriter, repo.MustBeNotEmpty)
1254+
1255+
m.Combo("/fork", reqRepoCodeReader).Get(repo.Fork).Post(web.Bind(forms.CreateRepoForm{}), repo.ForkPost)
12581256
}, reqSignIn, context.RepoAssignment, context.UnitTypes())
12591257

12601258
// Tags

services/context/repo.go

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -408,26 +408,6 @@ func repoAssignment(ctx *Context, repo *repo_model.Repository) {
408408
ctx.Data["IsEmptyRepo"] = ctx.Repo.Repository.IsEmpty
409409
}
410410

411-
// RepoIDAssignment returns a handler which assigns the repo to the context.
412-
func RepoIDAssignment() func(ctx *Context) {
413-
return func(ctx *Context) {
414-
repoID := ctx.ParamsInt64(":repoid")
415-
416-
// Get repository.
417-
repo, err := repo_model.GetRepositoryByID(ctx, repoID)
418-
if err != nil {
419-
if repo_model.IsErrRepoNotExist(err) {
420-
ctx.NotFound("GetRepositoryByID", nil)
421-
} else {
422-
ctx.ServerError("GetRepositoryByID", err)
423-
}
424-
return
425-
}
426-
427-
repoAssignment(ctx, repo)
428-
}
429-
}
430-
431411
// RepoAssignment returns a middleware to handle repository assignment
432412
func RepoAssignment(ctx *Context) context.CancelFunc {
433413
if _, repoAssignmentOnce := ctx.Data["repoAssignmentExecuted"]; repoAssignmentOnce {

templates/repo/header.tmpl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@
8282
{{/*else is not required here, because the button shouldn't link to any site if you can't create a fork*/}}
8383
{{end}}
8484
{{else if not $.UserAndOrgForks}}
85-
href="{{AppSubUrl}}/repo/fork/{{.ID}}"
85+
href="{{$.RepoLink}}/fork"
8686
{{else}}
8787
data-modal="#fork-repo-modal"
8888
{{end}}
@@ -103,7 +103,7 @@
103103
</div>
104104
{{if $.CanSignedUserFork}}
105105
<div class="divider"></div>
106-
<a href="{{AppSubUrl}}/repo/fork/{{.ID}}">{{ctx.Locale.Tr "repo.fork_to_different_account"}}</a>
106+
<a href="{{$.RepoLink}}/fork">{{ctx.Locale.Tr "repo.fork_to_different_account"}}</a>
107107
{{end}}
108108
</div>
109109
</div>

tests/integration/repo_fork_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,14 @@ func testRepoFork(t *testing.T, session *TestSession, ownerName, repoName, forkO
2929

3030
// Step2: click the fork button
3131
htmlDoc := NewHTMLParser(t, resp.Body)
32-
link, exists := htmlDoc.doc.Find("a.ui.button[href^=\"/repo/fork/\"]").Attr("href")
32+
link, exists := htmlDoc.doc.Find(`a.ui.button[href*="/fork"]`).Attr("href")
3333
assert.True(t, exists, "The template has changed")
3434
req = NewRequest(t, "GET", link)
3535
resp = session.MakeRequest(t, req, http.StatusOK)
3636

3737
// Step3: fill the form of the forking
3838
htmlDoc = NewHTMLParser(t, resp.Body)
39-
link, exists = htmlDoc.doc.Find("form.ui.form[action^=\"/repo/fork/\"]").Attr("action")
39+
link, exists = htmlDoc.doc.Find(`form.ui.form[action*="/fork"]`).Attr("action")
4040
assert.True(t, exists, "The template has changed")
4141
_, exists = htmlDoc.doc.Find(fmt.Sprintf(".owner.dropdown .item[data-value=\"%d\"]", forkOwner.ID)).Attr("data-value")
4242
assert.True(t, exists, fmt.Sprintf("Fork owner '%s' is not present in select box", forkOwnerName))
@@ -70,6 +70,6 @@ func TestRepoForkToOrg(t *testing.T) {
7070
req := NewRequest(t, "GET", "/user2/repo1")
7171
resp := session.MakeRequest(t, req, http.StatusOK)
7272
htmlDoc := NewHTMLParser(t, resp.Body)
73-
_, exists := htmlDoc.doc.Find("a.ui.button[href^=\"/repo/fork/\"]").Attr("href")
73+
_, exists := htmlDoc.doc.Find(`a.ui.button[href*="/fork"]`).Attr("href")
7474
assert.False(t, exists, "Forking should not be allowed anymore")
7575
}

0 commit comments

Comments
 (0)