Skip to content

Commit 8defca6

Browse files
GiteaBotlunny
andauthored
Fix possible pull request broken when leave the page immediately after clicking the update button (#34509) (#34607)
Backport #34509 by @lunny If user leaves the page, the context will become cancelled, so that the update process maybe terminal in an unexpected status. This PR haven't resolve the problem totally. It uses a background context to not cancel the update process even if the user leaved the pull request view page. Fix #31779 Co-authored-by: Lunny Xiao <[email protected]>
1 parent fac434d commit 8defca6

File tree

3 files changed

+22
-31
lines changed

3 files changed

+22
-31
lines changed

routers/api/v1/repo/pull.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"code.gitea.io/gitea/modules/base"
2424
"code.gitea.io/gitea/modules/git"
2525
"code.gitea.io/gitea/modules/gitrepo"
26+
"code.gitea.io/gitea/modules/graceful"
2627
"code.gitea.io/gitea/modules/log"
2728
"code.gitea.io/gitea/modules/setting"
2829
api "code.gitea.io/gitea/modules/structs"
@@ -1301,7 +1302,7 @@ func UpdatePullRequest(ctx *context.APIContext) {
13011302
// default merge commit message
13021303
message := fmt.Sprintf("Merge branch '%s' into %s", pr.BaseBranch, pr.HeadBranch)
13031304

1304-
if err = pull_service.Update(ctx, pr, ctx.Doer, message, rebase); err != nil {
1305+
if err = pull_service.Update(graceful.GetManager().ShutdownContext(), pr, ctx.Doer, message, rebase); err != nil {
13051306
if pull_service.IsErrMergeConflicts(err) {
13061307
ctx.APIError(http.StatusConflict, "merge failed because of conflict")
13071308
return

routers/web/repo/pull.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"code.gitea.io/gitea/modules/emoji"
2727
"code.gitea.io/gitea/modules/git"
2828
"code.gitea.io/gitea/modules/gitrepo"
29+
"code.gitea.io/gitea/modules/graceful"
2930
issue_template "code.gitea.io/gitea/modules/issue/template"
3031
"code.gitea.io/gitea/modules/log"
3132
"code.gitea.io/gitea/modules/setting"
@@ -980,7 +981,9 @@ func UpdatePullRequest(ctx *context.Context) {
980981
// default merge commit message
981982
message := fmt.Sprintf("Merge branch '%s' into %s", issue.PullRequest.BaseBranch, issue.PullRequest.HeadBranch)
982983

983-
if err = pull_service.Update(ctx, issue.PullRequest, ctx.Doer, message, rebase); err != nil {
984+
// The update process should not be cancelled by the user
985+
// so we set the context to be a background context
986+
if err = pull_service.Update(graceful.GetManager().ShutdownContext(), issue.PullRequest, ctx.Doer, message, rebase); err != nil {
984987
if pull_service.IsErrMergeConflicts(err) {
985988
conflictError := err.(pull_service.ErrMergeConflicts)
986989
flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{

services/pull/update.go

Lines changed: 16 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -41,22 +41,6 @@ func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.
4141
return fmt.Errorf("HeadBranch of PR %d is up to date", pr.Index)
4242
}
4343

44-
if rebase {
45-
defer func() {
46-
go AddTestPullRequestTask(TestPullRequestOptions{
47-
RepoID: pr.BaseRepo.ID,
48-
Doer: doer,
49-
Branch: pr.BaseBranch,
50-
IsSync: false,
51-
IsForcePush: false,
52-
OldCommitID: "",
53-
NewCommitID: "",
54-
})
55-
}()
56-
57-
return updateHeadByRebaseOnToBase(ctx, pr, doer)
58-
}
59-
6044
if err := pr.LoadBaseRepo(ctx); err != nil {
6145
log.Error("unable to load BaseRepo for %-v during update-by-merge: %v", pr, err)
6246
return fmt.Errorf("unable to load BaseRepo for PR[%d] during update-by-merge: %w", pr.ID, err)
@@ -74,6 +58,22 @@ func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.
7458
return fmt.Errorf("unable to load HeadRepo for PR[%d] during update-by-merge: %w", pr.ID, err)
7559
}
7660

61+
defer func() {
62+
go AddTestPullRequestTask(TestPullRequestOptions{
63+
RepoID: pr.BaseRepo.ID,
64+
Doer: doer,
65+
Branch: pr.BaseBranch,
66+
IsSync: false,
67+
IsForcePush: false,
68+
OldCommitID: "",
69+
NewCommitID: "",
70+
})
71+
}()
72+
73+
if rebase {
74+
return updateHeadByRebaseOnToBase(ctx, pr, doer)
75+
}
76+
7777
// TODO: FakePR: it is somewhat hacky, but it is the only way to "merge" at the moment
7878
// ideally in the future the "merge" functions should be refactored to decouple from the PullRequest
7979
// now use a fake reverse PR to switch head&base repos/branches
@@ -90,19 +90,6 @@ func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.
9090
}
9191

9292
_, err = doMergeAndPush(ctx, reversePR, doer, repo_model.MergeStyleMerge, "", message, repository.PushTriggerPRUpdateWithBase)
93-
94-
defer func() {
95-
go AddTestPullRequestTask(TestPullRequestOptions{
96-
RepoID: reversePR.HeadRepo.ID,
97-
Doer: doer,
98-
Branch: reversePR.HeadBranch,
99-
IsSync: false,
100-
IsForcePush: false,
101-
OldCommitID: "",
102-
NewCommitID: "",
103-
})
104-
}()
105-
10693
return err
10794
}
10895

0 commit comments

Comments
 (0)