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

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

brechtvl
Copy link
Contributor

@brechtvl brechtvl commented Apr 18, 2025

When viewing a file that the user can't edit because they can't write to
the branch, the new, upload, patch, edit and delete functionality is no
longer disabled.

If no user fork of the repository exists, there is now a page to create one.
It will automatically create a fork with a single branch matching the one
being viewed, and a unique repository name will be automatically picked.

When a fork exists, but it's archived, a mirror or the user can't write
code to it, there will instead be a message explaining the situation.

If the usable fork exists, a message will appear at the top of the edit page
explaining that the changes will be applied to a branch in the fork. The
base repository branch will be pushed to a new branch to the fork, and
then the edits will be applied on top.

The suggestion to fork happens when accessing /_edit/, so that for
example online documentation can have an "edit this page" link to
the base repository that does the right thing.

Also includes changes to properly report errors when trying to commit
to a new branch that is protected, and when trying to commit to an
existing branch when choosing the new branch option.

Resolves #9017


fork

not_directly_editable

archived

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 18, 2025
@github-actions github-actions bot added modifies/translation modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files labels Apr 18, 2025
@lunny lunny added this to the 1.24.0 milestone Apr 18, 2025
@lunny
Copy link
Member

lunny commented Apr 18, 2025

1 A middle page may be necessary like GH did? Looks like GH will not display a fork form page and will fork at the backend.

image

2 The branches may be fixed as All branches to prevent to choose a wrong branch

3 Is that really necessary to allow user to fork to an organization when forking for the edit?

4 The edit button on README file of repository home page could be displayed.

5 Too many steps and it will make some beginner confusing.

@wxiaoguang wxiaoguang removed this from the 1.24.0 milestone Apr 21, 2025
@brechtvl brechtvl marked this pull request as draft April 22, 2025 01:15
When viewing a file that the user can't edit because they can't write to
the branch, the edit button is no longer disabled.

If no user fork of the repository exists, there is now a page create one.
It will automatically create a fork with a single branch matching the one
being viewed, and a unique repository name will be automatically picked.

If the fork exists, an message will explain at the top of the edit page
explaining that the changes will be applied to a branch in the fork. The
base repository branch will be pushed to a new branch to the fork, and
then the edits will be applied on top.

This all happens when accessing /_edit/, so that for example online
documentation can have an "edit this page" link to the base repository
that does the right thing.
@brechtvl brechtvl changed the title Redirect to fork or propose to create one when editing files Edit file workflow for creating a fork and proposing changes Apr 22, 2025
@brechtvl
Copy link
Contributor Author

I rewrote the whole thing, which should hopefully address all the comments.

I think the permissions checking is rather confusing now. Previously ctx.Repo.CanWriteToBranch, ctx.Repo.CanEnableEditor and ctx.Repo.MustBeEditable were easy to understand, but now that two repositories are involved it's no so simple anymore. Maybe I should add a ctx.Repo.UserFork into the context, so that these functions can use it? Suggestions welcome.

@brechtvl brechtvl marked this pull request as ready for review April 22, 2025 18:40
@github-actions github-actions bot added the modifies/api This PR adds API routes or modifies them label Apr 23, 2025
@lunny
Copy link
Member

lunny commented Apr 23, 2025

Fork repository was created after clicking the fork the repository button. But it redirects to 404 . And when I re edit the file,
image

@lunny lunny added this to the 1.24.0 milestone Apr 23, 2025
@wxiaoguang wxiaoguang removed this from the 1.24.0 milestone Apr 23, 2025
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Apr 23, 2025

The "fork" logic has too much legacy patched code, it seems that it is not good to introduce more patches, instead maybe it needs refactoring ....

And I think it needs tests to make sure the logic is overall right.

Do we have enough time to take it into 1.24? (TBH I guess no)

@brechtvl
Copy link
Contributor Author

Fork repository was created after clicking the fork the repository button. But it redirects to 404 .

Fixed now, with test.

The "fork" logic has too much legacy patched code, it seems that it is not good to introduce more patches, instead maybe it needs refactoring ....

Ok, I'm not familiar enough with the overall code to see what is legacy here. Some hints or help would be appreciated.

Do we have enough time to take it into 1.24? (TBH I guess no)

I'm not in a rush to put it in 1.24.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files modifies/translation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Creating fork and submit PR to origin automatically when editing file from UI with read-only permission
4 participants