fix and add tests

Author: wxiaoguang

routers/web/repo/editor.go

@@ -144,7 +144,7 @@ func prepareEditorCommitSubmittedForm[T forms.CommitCommonFormInterface](ctx *co
 
 	fromBaseBranch := ctx.FormString("from_base_branch")
 	if fromBaseBranch != "" {
-		err = editorPushBranchToForkedRepository(ctx, ctx.Doer, ctx.Repo.Repository.BaseRepo, fromBaseBranch, ctx.Repo.Repository, ctx.Repo.RefFullName.BranchName())
+		err = editorPushBranchToForkedRepository(ctx, ctx.Doer, ctx.Repo.Repository.BaseRepo, fromBaseBranch, ctx.Repo.Repository, targetBranchName)
 		if err != nil {
 			log.Error("Unable to editorPushBranchToForkedRepository: %v", err)
 			ctx.JSONError(ctx.Tr("repo.editor.fork_failed_to_push_branch", targetBranchName))

services/context/repo.go

@@ -121,7 +121,7 @@ func PrepareCommitFormOptions(ctx *Context, doer *user_model.User, targetRepo *r
 		}
 		// now, we get our own forked repo; it must be writable by us.
 	}
-	submitToOriginRepo := targetRepo.ID == originRepo.ID
+	submitToForkedRepo := targetRepo.ID != originRepo.ID
 	err := targetRepo.GetBaseRepo(ctx)
 	if err != nil {
 		return nil, err
@@ -147,7 +147,7 @@ func PrepareCommitFormOptions(ctx *Context, doer *user_model.User, targetRepo *r
 		return nil, err
 	}
 
-	canCommitToBranch := submitToOriginRepo && targetRepo.CanEnableEditor() && canPushWithProtection
+	canCommitToBranch := !submitToForkedRepo /* same repo */ && targetRepo.CanEnableEditor() && canPushWithProtection
 	if protectionRequireSigned {
 		canCommitToBranch = canCommitToBranch && willSign
 	}
@@ -157,7 +157,7 @@ func PrepareCommitFormOptions(ctx *Context, doer *user_model.User, targetRepo *r
 
 	cfb := &CommitFormOptions{
 		TargetRepo:        targetRepo,
-		WillSubmitToFork:  !submitToOriginRepo,
+		WillSubmitToFork:  submitToForkedRepo,
 		CanCommitToBranch: canCommitToBranch,
 		UserCanPush:       canPushWithProtection,
 		RequireSigned:     protectionRequireSigned,
@@ -168,15 +168,17 @@ func PrepareCommitFormOptions(ctx *Context, doer *user_model.User, targetRepo *r
 		CanCreatePullRequest:     canCreatePullRequest,
 		CanCreateBasePullRequest: canCreateBasePullRequest,
 	}
-	editorAction, editorPathParamRemaining := ctx.PathParam("editor_action"), ctx.PathParam("*")
+	editorAction := ctx.PathParam("editor_action")
+	editorPathParamRemaining := util.PathEscapeSegments(branchName) + "/" + util.PathEscapeSegments(ctx.Repo.TreePath)
+	if submitToForkedRepo {
+		// there is only "default branch" in forked repo, we will use "from_base_branch" to get a new branch from base repo
+		editorPathParamRemaining = util.PathEscapeSegments(targetRepo.DefaultBranch) + "/" + util.PathEscapeSegments(ctx.Repo.TreePath) + "?from_base_branch=" + url.QueryEscape(branchName)
+	}
 	if editorAction == "_cherrypick" {
 		cfb.TargetFormAction = targetRepo.Link() + "/" + editorAction + "/" + ctx.PathParam("sha") + "/" + editorPathParamRemaining
 	} else {
 		cfb.TargetFormAction = targetRepo.Link() + "/" + editorAction + "/" + editorPathParamRemaining
 	}
-	if originRepo.ID != targetRepo.ID {
-		cfb.TargetFormAction += "?from_base_branch=" + url.QueryEscape(branchName)
-	}
 	return cfb, nil
 }
 

tests/integration/editor_test.go

@@ -342,14 +342,15 @@ index 0000000000..bbbbbbbbbb
 }
 
 func forkToEdit(t *testing.T, session *TestSession, owner, repo, operation, branch, filePath string) {
-	// Attempt to edit file
+	// attempt to edit a file, see the guideline page
 	req := NewRequest(t, "GET", path.Join(owner, repo, operation, branch, filePath))
 	resp := session.MakeRequest(t, req, http.StatusOK)
+	assert.Contains(t, resp.Body.String(), "Fork Repository to Propose Changes")
 
-	// Fork
+	// fork the repository
 	req = NewRequestWithValues(t, "POST", path.Join(owner, repo, "_fork", branch), map[string]string{"_csrf": GetUserCSRFToken(t, session)})
 	resp = session.MakeRequest(t, req, http.StatusOK)
-	assert.Equal(t, `{"redirect":""}`, strings.TrimSpace(resp.Body.String()))
+	assert.JSONEq(t, `{"redirect":""}`, resp.Body.String())
 }
 
 func testForkToEditFile(t *testing.T, session *TestSession, user, owner, repo, branch, filePath string) {
@@ -387,6 +388,37 @@ func testForkToEditFile(t *testing.T, session *TestSession, user, owner, repo, b
 	// Check the existence of the forked repo with unique name
 	req = NewRequestf(t, "GET", "/%s/%s-1", user, repo)
 	session.MakeRequest(t, req, http.StatusOK)
+
+	// the base repo's edit form should have the correct action and upload links (pointing to the forked repo)
+	req = NewRequest(t, "GET", path.Join(owner, repo, "_upload", branch, filePath))
+	resp = session.MakeRequest(t, req, http.StatusOK)
+	htmlDoc := NewHTMLParser(t, resp.Body)
+
+	form := htmlDoc.doc.Find(".form-fetch-action")
+	formAction := form.AttrOr("action", "")
+	assert.Equal(t, fmt.Sprintf("/%s/%s-1/_upload/%s/%s?from_base_branch=%s", user, repo, branch, filePath, branch), formAction)
+	uploadLink := form.Find(".dropzone").AttrOr("data-link-url", "")
+	assert.Equal(t, fmt.Sprintf("/%s/%s-1/upload-file", user, repo), uploadLink)
+	newBranchName := form.Find("input[name=new_branch_name]").AttrOr("value", "")
+	assert.Equal(t, user+"-patch-1", newBranchName)
+	commitChoice := form.Find("input[name=commit_choice][checked]").AttrOr("value", "")
+	assert.Equal(t, "commit-to-new-branch", commitChoice)
+	lastCommit := form.Find("input[name=last_commit]").AttrOr("value", "")
+	assert.NotEmpty(t, lastCommit)
+
+	// change a file in the forked repo
+	req = NewRequestWithValues(t, "POST", formAction,
+		map[string]string{
+			"_csrf":           GetUserCSRFToken(t, session),
+			"last_commit":     lastCommit,
+			"tree_path":       filePath,
+			"content":         "new content in fork",
+			"commit_choice":   commitChoice,
+			"new_branch_name": newBranchName,
+		},
+	)
+	resp = session.MakeRequest(t, req, http.StatusOK)
+	assert.Equal(t, fmt.Sprintf("/%s/%s/compare/%s...%s/%s-1:%s-patch-1", owner, repo, branch, user, repo, user), test.RedirectURL(resp))
 }
 
 func TestForkToEditFile(t *testing.T) {