mirror of
				https://github.com/go-gitea/gitea.git
				synced 2025-10-30 06:38:37 -04:00 
			
		
		
		
	Fix possible pull request broken when leave the page immediately after clicking the update button (#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 #31779
This commit is contained in:
		| @@ -23,6 +23,7 @@ import ( | |||||||
| 	"code.gitea.io/gitea/modules/base" | 	"code.gitea.io/gitea/modules/base" | ||||||
| 	"code.gitea.io/gitea/modules/git" | 	"code.gitea.io/gitea/modules/git" | ||||||
| 	"code.gitea.io/gitea/modules/gitrepo" | 	"code.gitea.io/gitea/modules/gitrepo" | ||||||
|  | 	"code.gitea.io/gitea/modules/graceful" | ||||||
| 	"code.gitea.io/gitea/modules/log" | 	"code.gitea.io/gitea/modules/log" | ||||||
| 	"code.gitea.io/gitea/modules/setting" | 	"code.gitea.io/gitea/modules/setting" | ||||||
| 	api "code.gitea.io/gitea/modules/structs" | 	api "code.gitea.io/gitea/modules/structs" | ||||||
| @@ -1301,7 +1302,7 @@ func UpdatePullRequest(ctx *context.APIContext) { | |||||||
| 	// default merge commit message | 	// default merge commit message | ||||||
| 	message := fmt.Sprintf("Merge branch '%s' into %s", pr.BaseBranch, pr.HeadBranch) | 	message := fmt.Sprintf("Merge branch '%s' into %s", pr.BaseBranch, pr.HeadBranch) | ||||||
|  |  | ||||||
| 	if err = pull_service.Update(ctx, pr, ctx.Doer, message, rebase); err != nil { | 	if err = pull_service.Update(graceful.GetManager().ShutdownContext(), pr, ctx.Doer, message, rebase); err != nil { | ||||||
| 		if pull_service.IsErrMergeConflicts(err) { | 		if pull_service.IsErrMergeConflicts(err) { | ||||||
| 			ctx.APIError(http.StatusConflict, "merge failed because of conflict") | 			ctx.APIError(http.StatusConflict, "merge failed because of conflict") | ||||||
| 			return | 			return | ||||||
|   | |||||||
| @@ -27,6 +27,7 @@ import ( | |||||||
| 	"code.gitea.io/gitea/modules/fileicon" | 	"code.gitea.io/gitea/modules/fileicon" | ||||||
| 	"code.gitea.io/gitea/modules/git" | 	"code.gitea.io/gitea/modules/git" | ||||||
| 	"code.gitea.io/gitea/modules/gitrepo" | 	"code.gitea.io/gitea/modules/gitrepo" | ||||||
|  | 	"code.gitea.io/gitea/modules/graceful" | ||||||
| 	issue_template "code.gitea.io/gitea/modules/issue/template" | 	issue_template "code.gitea.io/gitea/modules/issue/template" | ||||||
| 	"code.gitea.io/gitea/modules/log" | 	"code.gitea.io/gitea/modules/log" | ||||||
| 	"code.gitea.io/gitea/modules/setting" | 	"code.gitea.io/gitea/modules/setting" | ||||||
| @@ -986,7 +987,9 @@ func UpdatePullRequest(ctx *context.Context) { | |||||||
| 	// default merge commit message | 	// default merge commit message | ||||||
| 	message := fmt.Sprintf("Merge branch '%s' into %s", issue.PullRequest.BaseBranch, issue.PullRequest.HeadBranch) | 	message := fmt.Sprintf("Merge branch '%s' into %s", issue.PullRequest.BaseBranch, issue.PullRequest.HeadBranch) | ||||||
|  |  | ||||||
| 	if err = pull_service.Update(ctx, issue.PullRequest, ctx.Doer, message, rebase); err != nil { | 	// The update process should not be cancelled by the user | ||||||
|  | 	// so we set the context to be a background context | ||||||
|  | 	if err = pull_service.Update(graceful.GetManager().ShutdownContext(), issue.PullRequest, ctx.Doer, message, rebase); err != nil { | ||||||
| 		if pull_service.IsErrMergeConflicts(err) { | 		if pull_service.IsErrMergeConflicts(err) { | ||||||
| 			conflictError := err.(pull_service.ErrMergeConflicts) | 			conflictError := err.(pull_service.ErrMergeConflicts) | ||||||
| 			flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{ | 			flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{ | ||||||
|   | |||||||
| @@ -41,22 +41,6 @@ func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model. | |||||||
| 		return fmt.Errorf("HeadBranch of PR %d is up to date", pr.Index) | 		return fmt.Errorf("HeadBranch of PR %d is up to date", pr.Index) | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
| 	if rebase { |  | ||||||
| 		defer func() { |  | ||||||
| 			go AddTestPullRequestTask(TestPullRequestOptions{ |  | ||||||
| 				RepoID:      pr.BaseRepo.ID, |  | ||||||
| 				Doer:        doer, |  | ||||||
| 				Branch:      pr.BaseBranch, |  | ||||||
| 				IsSync:      false, |  | ||||||
| 				IsForcePush: false, |  | ||||||
| 				OldCommitID: "", |  | ||||||
| 				NewCommitID: "", |  | ||||||
| 			}) |  | ||||||
| 		}() |  | ||||||
|  |  | ||||||
| 		return updateHeadByRebaseOnToBase(ctx, pr, doer) |  | ||||||
| 	} |  | ||||||
|  |  | ||||||
| 	if err := pr.LoadBaseRepo(ctx); err != nil { | 	if err := pr.LoadBaseRepo(ctx); err != nil { | ||||||
| 		log.Error("unable to load BaseRepo for %-v during update-by-merge: %v", pr, err) | 		log.Error("unable to load BaseRepo for %-v during update-by-merge: %v", pr, err) | ||||||
| 		return fmt.Errorf("unable to load BaseRepo for PR[%d] during update-by-merge: %w", pr.ID, err) | 		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. | |||||||
| 		return fmt.Errorf("unable to load HeadRepo for PR[%d] during update-by-merge: %w", pr.ID, err) | 		return fmt.Errorf("unable to load HeadRepo for PR[%d] during update-by-merge: %w", pr.ID, err) | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
|  | 	defer func() { | ||||||
|  | 		go AddTestPullRequestTask(TestPullRequestOptions{ | ||||||
|  | 			RepoID:      pr.BaseRepo.ID, | ||||||
|  | 			Doer:        doer, | ||||||
|  | 			Branch:      pr.BaseBranch, | ||||||
|  | 			IsSync:      false, | ||||||
|  | 			IsForcePush: false, | ||||||
|  | 			OldCommitID: "", | ||||||
|  | 			NewCommitID: "", | ||||||
|  | 		}) | ||||||
|  | 	}() | ||||||
|  |  | ||||||
|  | 	if rebase { | ||||||
|  | 		return updateHeadByRebaseOnToBase(ctx, pr, doer) | ||||||
|  | 	} | ||||||
|  |  | ||||||
| 	// TODO: FakePR: it is somewhat hacky, but it is the only way to "merge" at the moment | 	// TODO: FakePR: it is somewhat hacky, but it is the only way to "merge" at the moment | ||||||
| 	// ideally in the future the "merge" functions should be refactored to decouple from the PullRequest | 	// ideally in the future the "merge" functions should be refactored to decouple from the PullRequest | ||||||
| 	// now use a fake reverse PR to switch head&base repos/branches | 	// 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. | |||||||
| 	} | 	} | ||||||
|  |  | ||||||
| 	_, err = doMergeAndPush(ctx, reversePR, doer, repo_model.MergeStyleMerge, "", message, repository.PushTriggerPRUpdateWithBase) | 	_, err = doMergeAndPush(ctx, reversePR, doer, repo_model.MergeStyleMerge, "", message, repository.PushTriggerPRUpdateWithBase) | ||||||
|  |  | ||||||
| 	defer func() { |  | ||||||
| 		go AddTestPullRequestTask(TestPullRequestOptions{ |  | ||||||
| 			RepoID:      reversePR.HeadRepo.ID, |  | ||||||
| 			Doer:        doer, |  | ||||||
| 			Branch:      reversePR.HeadBranch, |  | ||||||
| 			IsSync:      false, |  | ||||||
| 			IsForcePush: false, |  | ||||||
| 			OldCommitID: "", |  | ||||||
| 			NewCommitID: "", |  | ||||||
| 		}) |  | ||||||
| 	}() |  | ||||||
|  |  | ||||||
| 	return err | 	return err | ||||||
| } | } | ||||||
|  |  | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user