Skip to content

Commit f3b4e4c

Browse files
lunnyGiteaBot
authored andcommitted
Fix possible pull request broken when leave the page immediately after clicking the update button (go-gitea#34509)
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 go-gitea#31779
1 parent e18eae7 commit f3b4e4c

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)