Skip to content

Edit file workflow for creating a fork and proposing changes #34240

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 23 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
f834dca
Edit file workflow for creating a fork and proposing changes
brechtvl Apr 18, 2025
a9c552e
Fix tests and lint
brechtvl Apr 22, 2025
6d23290
Also support new/upload/patch/delete operations, refactor permissions
brechtvl Apr 22, 2025
c74b5fc
Check for protected branch also when committing to new branch
brechtvl Apr 23, 2025
64fa570
Add tests
brechtvl Apr 23, 2025
4da881b
Fix wrong redirect URL after fork, add test
brechtvl Apr 23, 2025
f29e486
Move fork to edit logic to a separate file
brechtvl Apr 23, 2025
a1e1730
Remove change from earlier version of the implementation
brechtvl Apr 23, 2025
d3f4f9d
Don't allow committing to existing branch when choosing new branch
brechtvl Apr 24, 2025
bc2d5aa
Share more checks with regular fork code, no reason not to
brechtvl Apr 24, 2025
333ff0e
Add a few more tests for failure cases
brechtvl Apr 24, 2025
b51ff01
Fix broken test due to stricter new branch check
brechtvl Apr 24, 2025
2410c43
Compare repositories by ID, add Gitea copyright to new file
brechtvl Apr 24, 2025
6ba37a0
Fix missing diff preview, revert order to what it was before, add test
brechtvl Apr 24, 2025
f2fc205
Fix translation lookup error due to missing WontSignReason
brechtvl Apr 27, 2025
170d6b6
Disable new/edit/delete button when not signed in
brechtvl Apr 29, 2025
bf23a19
Merge branch 'main' into fork-on-edit
brechtvl May 6, 2025
14549b8
Address comments from wxiaoguang
brechtvl May 6, 2025
693e600
Remove unused ids in fork template
brechtvl May 6, 2025
e78c6c4
markRepositoryAsNonEmpty
wxiaoguang May 7, 2025
e92d6a2
Use ctx.ServerError instead of custom message
brechtvl May 7, 2025
6ff820a
Fix permission check for uploading files, move GetEditableRepository …
brechtvl May 7, 2025
eb10c4f
Further permission checks, centralized in CanEnableEditor
brechtvl May 13, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions routers/web/repo/editor.go
Original file line number Diff line number Diff line change
Expand Up @@ -847,7 +847,6 @@ func cleanUploadFileName(name string) string {
}

// UploadFileToServer upload file to server file dir not git
// This is independent of any repository, no repository permissions are checked to call this.
func UploadFileToServer(ctx *context.Context) {
file, header, err := ctx.Req.FormFile("file")
if err != nil {
Expand Down Expand Up @@ -887,7 +886,6 @@ func UploadFileToServer(ctx *context.Context) {
}

// RemoveUploadFileFromServer remove file from server file dir
// This is independent of any repository, no repository permissions are checked to call this.
func RemoveUploadFileFromServer(ctx *context.Context) {
form := web.GetForm(ctx).(*forms.RemoveUploadFileForm)
if len(form.File) == 0 {
Expand Down
70 changes: 13 additions & 57 deletions routers/web/repo/fork_to_edit.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@ import (
"path"

git_model "code.gitea.io/gitea/models/git"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
repo_module "code.gitea.io/gitea/modules/repository"
Expand All @@ -28,63 +26,17 @@ const (
tplForkFile templates.TplName = "repo/editor/fork"
)

// getEditRepository returns the repository where the actual edits will be written to.
// This may be a fork of the repository owned by the user. If no repository can be found
// for editing, nil is returned along with a message explaining why editing is not possible.
func getEditRepository(ctx *context.Context) (*repo_model.Repository, any, error) {
if context.CanWriteToBranch(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.BranchName) {
return ctx.Repo.Repository, nil, nil
}

// If we can't write to the branch, try find a user fork to create a branch in instead
userRepo, err := repo_model.GetUserFork(ctx, ctx.Repo.Repository.ID, ctx.Doer.ID)
if err != nil {
return nil, nil, fmt.Errorf("GetUserFork: %v", err)
}
if userRepo == nil {
return nil, nil, nil
}

// Load repository information
if err := userRepo.LoadOwner(ctx); err != nil {
return nil, nil, fmt.Errorf("LoadOwner: v", err)
}
if err := userRepo.GetBaseRepo(ctx); err != nil || userRepo.BaseRepo == nil {
if err != nil {
return nil, nil, fmt.Errorf("GetBaseRepo: %v", err)
}
return nil, nil, fmt.Errorf("GetBaseRepo: Expected a base repo for user fork")
}

// Check code unit, archiving and permissions.
if !userRepo.UnitEnabled(ctx, unit.TypeCode) {
return nil, ctx.Tr("repo.editor.fork_code_disabled", userRepo.FullName()), nil
}
if userRepo.IsArchived {
return nil, ctx.Tr("repo.editor.fork_is_archived", userRepo.FullName()), nil
}
permission, err := access_model.GetUserRepoPermission(ctx, userRepo, ctx.Doer)
if err != nil {
return nil, nil, fmt.Errorf("access_model.GetUserRepoPermission: %v", err)
}
if !permission.CanWrite(unit.TypeCode) {
return nil, ctx.Tr("repo.editor.fork_no_permission", userRepo.FullName()), nil
}

return userRepo, nil, nil
}

// getEditRepositoryOrFork returns the repository where the edits will be written to.
// If no repository is editable, redirects to a page to create a fork.
func getEditRepositoryOrFork(ctx *context.Context, editOperation string) *repo_model.Repository {
editRepo, notEditableMessage, err := getEditRepository(ctx)
editRepo, msg, err := context.GetEditableRepository(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.BranchName)
if err != nil {
ctx.ServerError("getEditRepository", err)
return nil
}
if editRepo == nil {
// No editable repository, suggest to create a fork
forkToEditFileCommon(ctx, editOperation, ctx.Repo.TreePath, notEditableMessage)
forkToEditFileCommon(ctx, editOperation, ctx.Repo.TreePath, msg)
ctx.HTML(http.StatusOK, tplForkFile)
return nil
}
Expand All @@ -97,7 +49,7 @@ func getEditRepositoryOrFork(ctx *context.Context, editOperation string) *repo_m
// getEditRepositoryOrError returns the repository where the edits will be written to.
// If no repository is editable, display an error.
func getEditRepositoryOrError(ctx *context.Context, tpl templates.TplName, form any) *repo_model.Repository {
editRepo, _, err := getEditRepository(ctx)
editRepo, _, err := context.GetEditableRepository(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.BranchName)
if err != nil {
ctx.ServerError("getEditRepository", err)
return nil
Expand Down Expand Up @@ -174,36 +126,40 @@ func markRepositoryAsNonEmpty(ctx *context.Context, editRepo *repo_model.Reposit
_ = repo_model.UpdateRepositoryCols(ctx, &repo_model.Repository{ID: editRepo.ID, IsEmpty: false}, "is_empty")
}

func forkToEditFileCommon(ctx *context.Context, editOperation, treePath string, notEditableMessage any) {
func forkToEditFileCommon(ctx *context.Context, editOperation, treePath string, msg context.RepositoryNotEditableMessage) {
treeNames, treePaths := getParentTreeFields(treePath)

ctx.Data["EditOperation"] = editOperation
ctx.Data["TreePath"] = treePath
ctx.Data["TreeNames"] = treeNames
ctx.Data["TreePaths"] = treePaths
ctx.Data["CanForkRepo"] = notEditableMessage == nil
ctx.Data["NotEditableMessage"] = notEditableMessage

canForkRepo := msg.Reason == ""
ctx.Data["CanForkRepo"] = canForkRepo
if !canForkRepo {
ctx.Data["NotEditableMessage"] = ctx.Tr(msg.Reason, msg.Repository)
}
}

func ForkToEditFilePost(ctx *context.Context) {
form := web.GetForm(ctx).(*forms.ForkToEditRepoFileForm)

editRepo, notEditableMessage, err := getEditRepository(ctx)
editRepo, msg, err := context.GetEditableRepository(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.BranchName)
if err != nil {
ctx.ServerError("getEditRepository", err)
return
}

// Fork repository, if it doesn't already exist
if editRepo == nil && notEditableMessage == nil {
if editRepo == nil && msg.Reason == "" {
forkRepo := forkRepositoryOrError(ctx, ctx.Doer, repo_service.ForkRepoOptions{
BaseRepo: ctx.Repo.Repository,
Name: getUniqueRepositoryName(ctx, ctx.Repo.Repository.Name),
Description: ctx.Repo.Repository.Description,
SingleBranch: ctx.Repo.BranchName,
}, tplForkFile, form)
if forkRepo == nil {
forkToEditFileCommon(ctx, form.EditOperation, form.TreePath, notEditableMessage)
forkToEditFileCommon(ctx, form.EditOperation, form.TreePath, msg)
ctx.HTML(http.StatusOK, tplForkFile)
return
}
Expand Down
2 changes: 1 addition & 1 deletion routers/web/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -1336,7 +1336,7 @@ func registerWebRoutes(m *web.Router) {
m.Group("", func() {
m.Post("/upload-file", repo.UploadFileToServer)
m.Post("/upload-remove", web.Bind(forms.RemoveUploadFileForm{}), repo.RemoveUploadFileFromServer)
}, context.MustBeAbleToUpload())
}, context.MustBeAbleToUpload(), context.MustHaveEditableRepository())

m.Group("/branches", func() {
m.Group("/_new", func() {
Expand Down
63 changes: 63 additions & 0 deletions services/context/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,58 @@ func CanWriteToBranch(ctx context.Context, user *user_model.User, repo *repo_mod
return issues_model.CanMaintainerWriteToBranch(ctx, permission, branch, user)
}

// RepositoryNotEditableMessage explains why a repository can not be edited by the user
type RepositoryNotEditableMessage struct {
Reason string
Repository string
}

// GetEditRepository returns the repository where the editor will actually write the
// the edits to. This may be a fork of the repository owned by the user. If no repository
// can be found for editing, either a reason or error is returned.
func GetEditableRepository(ctx context.Context, user *user_model.User, repo *repo_model.Repository, branch string) (*repo_model.Repository, RepositoryNotEditableMessage, error) {
if CanWriteToBranch(ctx, user, repo, branch) {
return repo, RepositoryNotEditableMessage{}, nil
}

// If we can't write to the branch, try find a user fork to create a branch in instead
userRepo, err := repo_model.GetUserFork(ctx, repo.ID, user.ID)
if err != nil {
return nil, RepositoryNotEditableMessage{}, fmt.Errorf("GetUserFork: %v", err)
}
if userRepo == nil {
return nil, RepositoryNotEditableMessage{}, nil
}

// Load repository information
if err := userRepo.LoadOwner(ctx); err != nil {
return nil, RepositoryNotEditableMessage{}, fmt.Errorf("LoadOwner: %v", err)
}
if err := userRepo.GetBaseRepo(ctx); err != nil || userRepo.BaseRepo == nil {
if err != nil {
return nil, RepositoryNotEditableMessage{}, fmt.Errorf("GetBaseRepo: %v", err)
}
return nil, RepositoryNotEditableMessage{}, errors.New("GetBaseRepo: Expected a base repo for user fork")
}

// Check code unit, archiving and permissions.
if !userRepo.UnitEnabled(ctx, unit_model.TypeCode) {
return nil, RepositoryNotEditableMessage{Reason: "repo.editor.fork_code_disabled", Repository: userRepo.FullName()}, nil
}
if userRepo.IsArchived {
return nil, RepositoryNotEditableMessage{Reason: "repo.editor.fork_is_archived", Repository: userRepo.FullName()}, nil
}
permission, err := access_model.GetUserRepoPermission(ctx, userRepo, user)
if err != nil {
return nil, RepositoryNotEditableMessage{}, fmt.Errorf("access_model.GetUserRepoPermission: %v", err)
}
if !permission.CanWrite(unit_model.TypeCode) {
return nil, RepositoryNotEditableMessage{Reason: "repo.editor.fork_no_permission", Repository: userRepo.FullName()}, nil
}

return userRepo, RepositoryNotEditableMessage{}, nil
}

// CanEnableEditor returns true if the web editor can be enabled for this repository,
// either by directly writing to the repository or to a user fork.
func (r *Repository) CanEnableEditor() bool {
Expand Down Expand Up @@ -118,6 +170,17 @@ func MustBeAbleToUpload() func(ctx *Context) {
}
}

// MustHaveEditableRepository checks that there exists a repository that can be written
// to by the user for editing.
func MustHaveEditableRepository() func(ctx *Context) {
return func(ctx *Context) {
editRepo, _, _ := GetEditableRepository(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.BranchName)
if editRepo == nil {
ctx.NotFound(nil)
}
}
}

// CanCommitToBranchResults represents the results of CanCommitToBranch
type CanCommitToBranchResults struct {
CanCommitToBranch bool
Expand Down